Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Policy Attachment Should Support a TargetRef with Gateway Class Name(s) #3175

Open
robscott opened this issue Jun 25, 2024 · 15 comments
Open
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.

Comments

@robscott
Copy link
Member

robscott commented Jun 25, 2024

What would you like to be added:
A way for policies to apply only to specific GatewayClasses. This would not be required, but it would be a field that policies could choose to include in their targetRef and would be recognized + supported by tools like gwctl when it was present.

For example, we might have a targetRef like this:

targetRefs:
- kind: Service
  name: foo
  gatewayClassName: bar

This would mean that this specific policy attached to Service foo would only be implemented by Gateways of bar class, and would be ignored by other Gateways.

Why this is needed:
We're seeing the rise of multi-implementation policies used with Gateway API - policies that are intended to be implemented by multiple GatewayClasses such as BackendTLSPolicy, BackendLBPolicy, and even some vendor-specific ones where the vendor actually has multiple implementations.

@robscott robscott added the kind/feature Categorizes issue or PR as related to a new feature. label Jun 25, 2024
@candita
Copy link
Contributor

candita commented Jul 8, 2024

Can you please change "ClassName(s)" to "gateway class name(s)"?

@arkodg
Copy link
Contributor

arkodg commented Jul 8, 2024

+1
adding this will make the attachment 1:1, and will allow implementations to safely add a ReasonTargetNotFound reason in the status in case a policy cannot attach to the target
relates to envoyproxy/gateway#2724

@robscott robscott changed the title Policy Attachment Should Support a TargetRef with ClassName(s) Policy Attachment Should Support a TargetRef with Gateway ClassName(s) Jul 9, 2024
@robscott robscott changed the title Policy Attachment Should Support a TargetRef with Gateway ClassName(s) Policy Attachment Should Support a TargetRef with Gateway Class Name(s) Jul 9, 2024
@arkodg
Copy link
Contributor

arkodg commented Jul 9, 2024

rethinking this one, the issue is tied to the implementation's inability to decide whether its meant to reconcile a policy or not. It doesnt have this problem for other gateway-api resources since they all link back to a gatewayClass parent which links to a controller/implementation. So would it be better to add a way to link back to the controller in the policy spec itself ?

@keithmattix
Copy link
Contributor

I'm torn because on one hand, this feels more like a policy all on its own (almost ReferenceGrantish?). The problem with that option is that it would require yet another optional resource that needs to be created. 🤔 Maybe it's the least bad option?

@candita
Copy link
Contributor

candita commented Jul 11, 2024

With Gateway API policies like BackendTLSPolicy and BackendLBPolicy, another use case would be for cluster admins to be empowered to create "global" policies that would be applied to specific GatewayClasses and their children.

On the other hand, other policies, like Kuadrant's AuthPolicy and RateLimitPolicy, can already target specific HTTPRoutes, or a set of listeners in a gateway. Why stop at GatewayClasses as targets? Check out Kuadrant's policy machinery. It uses a "Topology struct for modeling topologies of targetable network resources and corresponding attached policies". 😎

cc @maleck13 @guicassolato

@robscott
Copy link
Member Author

I'm realizing my issue description did not actually clearly convey what I meant. I actually meant a way for targetRefs to include an optional gatewayClassName field, for example:

targetRefs:
- kind: Service
  name: foo
  gatewayClassName: bar

This would mean that this specific policy attached to Service foo would only be implemented by Gateways of bar class, and would be ignored by other Gateways. Will rework my original description to make that intent clearer.

@mikemorris
Copy link
Contributor

mikemorris commented Jul 19, 2024

I think I kinda agree with @arkodg's line of thinking here but feel like we may be conflating two somewhat-related concerns:

  1. Should the controller for my implementation pay attention to this policy?
    • Currently, if a controller understands/is watching for a policy, it is generally assumed it should be respected. This seems mostly fine for implementation-specific policies, but centrally-defined policies like BackendTLSPolicy used by multiple implementations can make this more complicated.
  2. For which "clients" should a policy targeting some resource lacking a clear ancestry ownership line up to a given controller (such as Service, which can't be traced back the way HTTPRoute -> parentRef Gateway -> gatewayClassName GatewayClass -> controllerName can be) be applied?

1️⃣ (and by extension scoping an existing targetRef to gatewayClassName) is somewhat of a coarse way to achieve 2️⃣ , by implying that all Gateway clients of a given controller (or GatewayClass) should respect a policy. While this could be useful, I'm not sure if this is actually sufficiently granular, as it may be inadequate for some use cases where different Gateway clients of the same GatewayClass or controller should have different configuration, policy or priority when addressing a given backend (for example), and it's definitely inadequate for other cases of multiple-selection policy, such as AuthZ policy, where a service owner would want to configure specific authorization policies for different clients.

I'm more inclined to head in the direction @candita is suggesting, and think if we could design a secondary targeting structure more like Kuadrant's topology or what would be needed for the AuthZ policy case (like the multiple from clauses in Istio's AuthorizationPolicy as an example, scoping which clients are allowed to connect to backends associated with a given namespace/pod selector/targetRef), we would also be able to nicely solve the Gateway client policy selection in a more granular way.

@arkodg
Copy link
Contributor

arkodg commented Jul 19, 2024

Solution1

targetRefs:
- kind: HTTPRoute
  name: route1
  gatewayClassName: class1

Solution2

gatewayClassName: class1
targetRefs:
- kind: HTTPRoute
  name: route1

Although both options solve the multiple problems outlined in this issue, imo solution2 creates a stronger parent child link between a gatewayClass and policy

Untitled (3)

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 17, 2024
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle rotten
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Nov 16, 2024
@maleck13
Copy link
Contributor

/remove-lifecycle rotten

@k8s-ci-robot k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Nov 19, 2024
@arkodg
Copy link
Contributor

arkodg commented Nov 19, 2024

@guicassolato was this issue discussed in person at kubecon ?

@mikemorris
Copy link
Contributor

mikemorris commented Nov 20, 2024

Bumping this in the context of istio/istio#53991 - would the proposal in this issue conflict (or simply be confusing) with policy attachment patterns targeting a GatewayClass directly, which GEP-2649 seems to allow with the caveat that it's "tricky", might require a cluster-scoped policy resource (and thus likely only be usable for the cluster operator Chihiro persona) and that a GatewayClass paramsRef resource field may be more straightforward for this behavior?

Would our use case in Istio (cluster-wide default-deny AuthZ policy) be more appropriate for introducing and targeting the Mesh resource we've discussed for GAMMA but has sat in limbo without a clear motivating need?

/cc @ilrudie

@youngnick
Copy link
Contributor

We did discuss this in person, and I recorded my concerns about combinatorial complexity.

Personally, I feel like the pattern of making things broadly target some class of other object then having a field that says "oh, only the ones that roll up to this GatewayClass" smells funny.

I think for use cases like @mikemorris talks about above, it's way better to anchor the Policy to something that fits into an existing hierarchy.

Every dimension we add to Policy targeting produces an exponential effect on the overall complexity.

Already, it's possible to have a single Policy that targets multiple objects in multiple namespaces, and folks are also asking for label selectors - if we add this on top, then you can have a single policy that targets a label selector, but only for objects that roll up to a single GatewayClass. That's rapidly approaching the level of spooky-action-at-a-distance that is completely impenetrable to Ana the end user.

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
Development

No branches or pull requests

9 participants