search

kubernetes-sigs / gateway-api

Repository for the next iteration of composite service (e.g. Ingress) and load balancing APIs.
https://gateway-api.sigs.k8s.io
Apache License 2.0
1.78k stars 461 forks source link

Can `HTTPRoute` support global filters? #1973

Closed machine3 closed 1 week ago

machine3 commented 1 year ago

What would you like to be added:

Can HTTPRoute support global filters?

apiVersion: gateway.networking.k8s.io/v1beta1
kind: HTTPRoute
metadata:
  name: myroute
spec:
  parentRefs:
    - name: mygw
  rules:
    - matches:
        - path:
            type: PathPrefix
            value: /test1
      backendRefs:
        - name: test1
          port: 8080
    - matches:
        - path:
            type: PathPrefix
            value: /test2
      backendRefs:
        - name: test2
          port: 8080
  filters:  # Generate efficiency for all rules
    - type: Cors
      cors:
        allowOriginRegex:
          - '^http(s)?:\/\/localhost:[0-9]{4,5}$'
        allowHeaders:
          - origin
          - content-type
        maxAge: 1d

Why this is needed: For example, I want to add a cors filter under a HTTPRoute, which will take effect for all rules under the HTTPRoute.

howardjohn commented 1 year ago

IMO this type of syntax sugar is best handled by higher level tooling (helm, etc).

machine3 commented 1 year ago

IMO this type of syntax sugar is best handled by higher level tooling (helm, etc).

@howardjohn What I described before may not be very accurate, I updated it, please help me to have a look

howardjohn commented 1 year ago

I think I understood it the first time and still have the same opinion here. Its not worth the complexity for the gain given the same benefits can be achieved via tooling on top, IMO.

arkodg commented 1 year ago

+1 to this change, since it improves UX, there are a set of filters that will most likely be common to the HTTPRoute such as CORS, RateLimiting, Mirroring (all under ExtensionRef today) and some that will be specific to a rule (RequestHeaderModifier, RequestRedirect etc)

machine3 commented 1 year ago

I think I understood it the first time and still have the same opinion here. Its not worth the complexity for the gain given the same benefits can be achieved via tooling on top, IMO.

Although it is feasible for the upper layer to handle it, it would be better if the general capabilities of the api gateway can be achieved in the standard. Gloo edge has a similar function, they are called options cors, and from the usage scenarios, not only cors, such as ip black and white lists, etc., they The scope of effect is not just the dimension of routing rules. It is impossible for me to add an ip black and white list filter every time I create a routing rule. From the perspective of API, it is better to abstract out a lot of repeated definitions.

hzxuzhonghu commented 1 year ago

I think it makes sense to do it

howardjohn commented 1 year ago

IMO this is something where Policy Attachment makes sense, which already has the ability to insert at arbitrary points. I worry we are just re-inventing the same concept in two different ways. cc @youngnick

youngnick commented 1 year ago

This is one hundred percent an intended use for Policy Attachment, yes. Specifying a default Filter is a great use for an Inherited Policy.

machine3 commented 1 year ago

You mean using Policy Attachmen, like this?

apiVersion: networking.example.io/v1alpha1
kind: FiltersPolicy
metadata:
  name: myroute-filters
spec:
  default:
    filters:
      - type: Cors
        cors:
          allowOriginRegex:
            - '^http(s)?:\/\/localhost:[0-9]{4,5}$'
          allowHeaders:
            - origin
            - content-type
          maxAge: 1d
  targetRef:
    name: myroute
    kind: HTTPRoute

@youngnick @howardjohn

howardjohn commented 1 year ago

Yes you could even do it at the entire Gateway level

shaneutt commented 1 year ago

If I'm reading the conversation here right it seems if there's any action to take its improving documentation (of policy attachment) so that it's better understood that this can and should be accomplished at that level, and to that end it seems reasonable to accept:

/kind gep /kind documentation /triage accepted

However as this doesn't seem like something we must resolve before GA, we would consider this a lower priority than GA requirements until v1.0.0 is released:

/priority backlog

k8s-triage-robot commented 5 months ago

This issue has not been updated in over 1 year, and should be re-triaged.

You can:

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted

k8s-triage-robot commented 2 months ago

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

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

You can:

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

/lifecycle stale

k8s-triage-robot commented 1 month ago

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:

You can:

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

/lifecycle rotten

k8s-triage-robot commented 1 week ago

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

This bot triages issues according to the following rules:

You can:

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

/close not-planned

k8s-ci-robot commented 1 week ago

@k8s-triage-robot: Closing this issue, marking it as "Not Planned".

In response to [this](https://github.com/kubernetes-sigs/gateway-api/issues/1973#issuecomment-2371748411): >The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. > >This bot triages 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: >- Reopen this issue with `/reopen` >- Mark this issue as fresh with `/remove-lifecycle rotten` >- Offer to help out with [Issue Triage][1] > >Please send feedback to sig-contributor-experience at [kubernetes/community](https://github.com/kubernetes/community). > >/close not-planned > >[1]: https://www.kubernetes.dev/docs/guide/issue-triage/ Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.