gateway-api icon indicating copy to clipboard operation
gateway-api copied to clipboard

Match multiple methods in HTTPRouteMatch

Open tamalsaha opened this issue 2 years ago • 8 comments

What would you like to be added: https://github.com/kubernetes-sigs/gateway-api/blob/f8442221c60b186c23560232ffa65803a5d09278/apis/v1beta1/httproute_types.go#L546

It will be better DX to support multiple method matches in HTTPRouteMatch

Method []HTTPMethod json:"method,omitempty"``

Why this is needed:

While this can be done using multiple match rules, it is better DX to support []HTTPMethod. Also, the other matches (header, query params) support array. So, it will be also more consistent.

Similar issue also exists in GRPC api.

tamalsaha avatar Mar 08 '23 02:03 tamalsaha

I think this is a reasonable request, but unfortunately, it's a breaking API change, so we'll probably need to add a plural field (i.e. Methods instead of Method, with semantics for cross-populating Method into Methods etc). This sucks to some extent but we don't want to make a breaking API change for HTTPRoute at this point, sadly.

youngnick avatar Mar 08 '23 05:03 youngnick

The upstream reference on the right way to make a singular field plural is https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api_changes.md#making-a-singular-field-plural

youngnick avatar Mar 20 '23 22:03 youngnick

The upstream reference on the right way to make a singular field plural is https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api_changes.md#making-a-singular-field-plural

So it would seem this is doable, albeit it's not going to be simple or quick. Marking this as accepted and giving it a priority for now and we'll allow some time to see if anyone wants to take it on:

/remove-triage /priority backlog /help

shaneutt avatar Mar 20 '23 23:03 shaneutt

@shaneutt: This request has been marked as needing help from a contributor.

Guidelines

Please ensure that the issue body includes answers to the following questions:

  • Why are we solving this issue?
  • To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
  • Does this issue have zero to low barrier of entry?
  • How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed by commenting with the /remove-help command.

In response to this:

The upstream reference on the right way to make a singular field plural is https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api_changes.md#making-a-singular-field-plural

So it would seem this is doable, albeit it's not going to be simple or quick. Marking this as accepted and giving it a priority for now and we'll allow some time to see if anyone wants to take it on:

/remove-triage /priority backlog /help

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/test-infra repository.

k8s-ci-robot avatar Mar 20 '23 23:03 k8s-ci-robot

DRY - Example of how restricting a route to certain HTTP methods will look like after making method plural. After:

  rules:
  - backendRefs:
    - name: my-service
      port: 8080
    matches:
    - path:
        type: PathPrefix
        value: /api/my-service/
      # Per making-a-singular-field-plural above - that plural[0] must match the singular value.
      method: GET
      methods:
      - GET
      - HEAD
      - POST
      - PUT
      - DELETE
      - PATCH
      - OPTIONS

Now:

  rules:
  - backendRefs:
    - name: my-service
      port: 8080
    matches:
    - path:
        type: PathPrefix
        value: /api/my-service/
      method: GET
    - path:
        type: PathPrefix
        value: /api/my-service/
      method: HEAD
    - path:
        type: PathPrefix
        value: /api/my-service/
      method: POST
    - path:
        type: PathPrefix
        value: /api/my-service/
      method: PUT
    - path:
        type: PathPrefix
        value: /api/my-service/
      method: DELETE
    - path:
        type: PathPrefix
        value: /api/my-service/
      method: PATCH
    - path:
        type: PathPrefix
        value: /api/my-service/
      method: OPTIONS

mindw avatar Dec 07 '25 07:12 mindw

It's a little bit better than that; because making a required field optional is not a breaking change, we can make the method field optional, and codify some of the pluralisation rules into CEL (so you can't specify method: GET, but have POST be the first element of methods, for example).

As I said above, this is a change we can accept for the reasons that @mindw illustrates above. Someone just needs to sign up to do the pluralisation work. @mindw, happy for you to pick this up if you wish, I can help guide you through the process.

youngnick avatar Dec 09 '25 01:12 youngnick

Hi @youngnick

I’d like to work on this issue. I’ve gone through the details and the linked API change process for converting singular fields to plural.

Could you please assign this issue to me? I’m ready to follow the singular-to-plural API evolution guidelines, and I appreciate your offer to guide the process.

Thanks!

darshank188 avatar Dec 12 '25 01:12 darshank188

/assign @darshank188

youngnick avatar Dec 12 '25 02:12 youngnick