wouldnt this need more fields so it can also incorporate Kind and Group ?

      group: gateway.networking.k8s.io
      kind: HTTPRoute
      selector/matchLabels:
        app: foo

While it's possible to add this, I don't think it's needed.

The selector is checked only against routes and gateways anyways right now, and if a developer/admin wants to limit the matched policies to a specific kind it's possible to add the kind as a label.

as a project, we've tried to use strongly typed fields as much as possible, my suggestion would be to provide a kind and group field here along with the labelSelector field

I think a kind and group field as part of the target selector defeats the purpose here. If I add a kind and group field, then how would that work?

Let's say I want to attach to all of my routes (HTTPRoute, GRPCRoute, TLSRoute, and TCPRoute) to add some default setting. What do I specify in the kind field?

And, considering we enforce that the group field is always a single value using CEL, what should I as a user do with the group field? Fill it, and make sure it always matches the exactly one valid value? Leave it empty and assume that the default single value is being filled in?

reg attaching to all xRoutes, 4 policies could be created per xRoute or 1 policy could be attached to the Gateway

I think that since this API still needs some discussion, I will remove the targetSelector for now from the PR and keep only the plural targetRefs in this PR.

The target selectors can be done in a separate PR after the API has been designed and agreed upon.

sure raised #3607 to track this work

can you rebase ? the changes here are unintentional

done.

should we change this in a separate PR? It causes a lot of file changes.

This change didn't cause the file changes. I explicitly chmod-ed the yaml files which were marked as executable so that they wouldn't be.

This change only makes sure that new files being added are created without the executable bit set.

curious why we need this map ? if its being used to handle duplicates, any idea how we ended up with duplicates ?

For example:

There's a gateway with 3 different routes. You create a single policy resource that targets two of those three routes with two different targetRef elements.

This map is a way to prevent the policy resource from being added twice to the res array.

re-reading, its because we have multiple targetRefs, can we execute res = append(res, policy) right after

for _, currPolicy := range backendTrafficPolicies {
    res = append(res, curPolicy)
    ....

and avoid extra memory

It's not that simple. The code has two loops, the first handling the policies that target routes and the second handling the targets that target gateways. Once we have multiple targetRefs, it's possible for the same policy to target both a route and a different gateway.

The second loop needs to work on the same deep copy of the policy as the first loop, otherwise the status section of the policy is not correctly updated. If we do as you suggest in the first loop, then the second loop would need to go over all of the content of res and find the correct policy instance if its there. The easy way to do this is like the code does right now.

ah thanks for explaining
do we want to support a policy targeting a Gateway and HTTPRoute at the same time ? does our library gracefully handle this case when it comes to defaults and overrides ?

It's no different than two different policies targeting both the gateway and the route. The plural targetRefs is just a form of syntactic sugar.

nit: prefer

ancestorRefs := getAncestorRefs(policy)
ancestorRefs = append(ancestorRefs, parent)

simplifies reading the code w/o having to read the details in getAncestorRefs