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

Can HTTPRoute support global filters? #1973

Closed
machine3 opened this issue Apr 25, 2023 · 16 comments
Closed

Can HTTPRoute support global filters? #1973

machine3 opened this issue Apr 25, 2023 · 16 comments
Labels
kind/documentation Categorizes issue or PR as related to documentation. kind/feature Categorizes issue or PR as related to a new feature. kind/gep PRs related to Gateway Enhancement Proposal(GEP) lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. priority/backlog Higher priority than priority/awaiting-more-evidence.

Comments

@machine3
Copy link

machine3 commented Apr 25, 2023

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.

@machine3 machine3 added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 25, 2023
@howardjohn
Copy link
Contributor

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

@shaneutt shaneutt added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Apr 25, 2023
@machine3
Copy link
Author

machine3 commented Apr 26, 2023

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
Copy link
Contributor

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
Copy link
Contributor

arkodg commented Apr 26, 2023

+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
Copy link
Author

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
Copy link
Member

I think it makes sense to do it

@howardjohn
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

Yes you could even do it at the entire Gateway level

@shaneutt
Copy link
Member

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-ci-robot k8s-ci-robot added kind/gep PRs related to Gateway Enhancement Proposal(GEP) kind/documentation Categorizes issue or PR as related to documentation. triage/accepted Indicates an issue or PR is ready to be actively worked on. priority/backlog Higher priority than priority/awaiting-more-evidence. labels Apr 28, 2023
@shaneutt shaneutt removed the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Apr 28, 2023
@k8s-triage-robot
Copy link

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

You can:

  • Confirm that this issue is still relevant with /triage accepted (org members only)
  • Close this issue with /close

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

/remove-triage accepted

@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. and removed triage/accepted Indicates an issue or PR is ready to be actively worked on. labels Apr 27, 2024
@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 Jul 26, 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 Aug 25, 2024
@k8s-triage-robot
Copy link

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

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

/close not-planned

@k8s-ci-robot k8s-ci-robot closed this as not planned Won't fix, can't repro, duplicate, stale Sep 24, 2024
@k8s-ci-robot
Copy link
Contributor

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

In response to this:

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

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

/close not-planned

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/documentation Categorizes issue or PR as related to documentation. kind/feature Categorizes issue or PR as related to a new feature. kind/gep PRs related to Gateway Enhancement Proposal(GEP) lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. priority/backlog Higher priority than priority/awaiting-more-evidence.
Projects
None yet
Development

No branches or pull requests

8 participants