add overriding condition to BackendTrafficPolicy and SecurityPolicy

[zhaohuabing]

fix #2055

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.50%. Comparing base (6d8337e) to head (1bf5654).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2684      +/-   ##
==========================================
+ Coverage   63.34%   63.50%   +0.16%     
==========================================
  Files         123      123              
  Lines       19979    20056      +77     
==========================================
+ Hits        12655    12736      +81     
+ Misses       6513     6510       -3     
+ Partials      811      810       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[arkodg]

thanks for picking this one up @zhaohuabing

• can the PR also apply the same logic for SecurityPolicy ?
• as part of this PR can we also fix the logic here
  

  gateway/internal/gatewayapi/backendtrafficpolicy.go

  Line 364 in 72fadb7

  for _, r := range http.Routes {

  
  as pointed out by you earlier, its adoping Merge semantics, when this should be a Replace i.e. if that route has an existing policy attached (maybe computed using the routeMap, we should skip applying any setting, rather than checking which is nil)

[zhaohuabing]

can the PR also apply the same logic for SecurityPolicy ?

Yes, I originally planned to apply this to SecurityPolicy when this one is approved. But since you already mentioned this, I'm going to do it in this PR.

as pointed out by you earlier, its adoping Merge semantics, when this should be a Replace i.e. if that route has an existing policy attached (maybe computed using the routeMap, we should skip applying any setting, rather than checking which is nil)

I thought it was intentional. For example, a default global config can be defined at the Gateway level, and app developers can fine-tune the configurations at the route level.

The benefit of having a global config on the top is that the app developers don't have to redefine every setting at the roue level, but only these they really want to modify. The downside is that it's difficult to figure out wich setting comes form which policy.

[arkodg]

I thought it was intentional. For example, a default global config can be defined at the Gateway level, and app developers can fine-tune the configurations at the route level.
The benefit of having a global config on the top is that the app developers don't have to redefine every setting at the roue level, but only these they really want to modify. The downside is that it's difficult to figure out wich setting comes from which policy.

@zhaohuabing Replace by the most specific resource is the default https://gateway.envoyproxy.io/v0.6.0/design/security-policy/#design-decisions , and support for Merge will hopefully be tackled in the near future with #1934

[zhaohuabing]

@zhaohuabing Replace by the most specific resource is the default https://gateway.envoyproxy.io/v0.6.0/design/security-policy/#design-decisions , and support for Merge will hopefully be tackled in the near future with #1934

@arkodg Got it! Do you mind if I fix this in a separate PR?

[arkodg]

@zhaohuabing Replace by the most specific resource is the default https://gateway.envoyproxy.io/v0.6.0/design/security-policy/#design-decisions , and support for Merge will hopefully be tackled in the near future with #1934

@arkodg Got it! Do you mind if I fix this in a separate PR?

Sure sgtm !

[arkodg]

LGTM thanks !