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

Adds Gateway API HTTPRoute integration #39

Merged
merged 2 commits into from
Feb 3, 2022
Merged

Conversation

jjaferson
Copy link
Contributor

Adds integration to Gateway API to generate HTTPRoute

Command to generate Gateway API routing integration from Open API specification:

kuadrantctl generate gatewayapi httproute 

Example:

---
openapi: "3.0.0"
info:
  title: "Pet Store API"
  version: "1.0.0"
servers:
  - url: https://toplevel.example.io/v1
paths:
  /cat:
    get:
      parameters:
        - in: header
          name: X-username
          schema:
            type: string
      operationId: "getCat"
      responses:
        405:
          description: "invalid input"
    post:
      operationId: "postCat"
      responses:
        405:
          description: "invalid input"
  /dog:
    get:
      operationId: "getDog"
      responses:
        405:
          description: "invalid input"

Running the defined parameters below --namespace myns, --oas ./examples/oas3/petstore.yaml, --public-host www.kuadrant.io, --service-name myservice and --gateway kuadrant-gateway

kuadrantctl generate gatewayapi httproute --namespace myns --oas ./examples/oas3/petstore.yaml --public-host www.kuadrant.io --service-name myservice --gateway kuadrant-gateway

The CLI will generate the result below:

{
   "kind":"HTTPRoute",
   "apiVersion":"gateway.networking.k8s.io/v1alpha2",
   "metadata":{
      "creationTimestamp":null
   },
   "spec":{
      "hostnames":[
         "www.kuadrant.io"
      ],
      "rules":[
         {
            "matches":[
               {
                  "path":{
                     "type":"PathPrefix",
                     "value":"/cat"
                  },
                  "headers":[
                     {
                        "type":"Exact",
                        "name":"X-username",
                        "value":""
                     }
                  ],
                  "method":"GET"
               },
               {
                  "path":{
                     "type":"PathPrefix",
                     "value":"/cat"
                  },
                  "method":"POST"
               },
               {
                  "path":{
                     "type":"PathPrefix",
                     "value":"/dog"
                  },
                  "method":"GET"
               }
            ],
            "backendRefs":[
               {
                  "name":"myservice.myns.svc",
                  "port":80
               }
            ]
         }
      ]
   },
   "status":{
      "parents":null
   }
}

cmd/generate_gatewayapi_httproute.go Outdated Show resolved Hide resolved
cmd/generate_gatewayapi_httproute.go Show resolved Hide resolved
for verb, operation := range pathItem.Operations() {
pathValue := path
path := &gatewayapiv1alpha2.HTTPPathMatch{
Type: &pathMatchPathPrefix,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why prefix and not exact match?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went for Prefix instead of ExactMatch as it gives the ability for other requests that don't fully match with further HTTPRoutes matches to fall back.

This is something that the Gateway API defines by allowing many backend services to be specified in a HTTPRoute and the most specific match will take precedence.
https://gateway-api.sigs.k8s.io/v1alpha2/guides/http-routing/

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand what is the meaning of both match types. My question is about why one or the other. In other words, what would a customer expect from it? The customer specifies

GET /toy

If a request like GET /toy/something arrives, does the customer expect to get hit by that request?

PS: regarding the most specific match will take precedence I have not read anything about that. What I understand is that the first rule that matches will apply its routing definition.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in this case, if there isn't any HTTPRoute created for GET /toy/something the traffic be redirected to GET /toy so the request wouldn't be rejected, I think it should be down to the API designer to decide, we could set a default value eg. : ExactMatch and then allow the value to be changed via a parameter (GET /toy?exactPath=false) defined in the request, not sure if we can make it more flexible using OAS.

I am trying to make it as flexible as possible, but it makes sense to set a hard specification and allow define traffic to an exact path.

That is what is described in the Gateway API guides:

The most specific match will take precedence which means that any traffic with the env: canary header will be forwarded to bar-svc-canary and if the header is missing or not canary then it'll be forwarded to bar-svc.

apiVersion: gateway.networking.k8s.io/v1alpha2
kind: HTTPRoute
metadata:
  name: bar-route
spec:
  parentRefs:
  - name: example-gateway
  hostnames:
  - "bar.example.com"
  rules:
  - matches:
    - headers:
      - type: Exact
        name: env
        value: canary
    backendRefs:
    - name: bar-svc-canary
      port: 8080
  - backendRefs:
    - name: bar-svc
      port: 8080

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in this case, if there isn't any HTTPRoute created for GET /toy/something the traffic be redirected to GET /toy so the request wouldn't be rejected, I think it should be down to the API designer to decide, we could set a default value eg. : ExactMatch and then allow the value to be changed via a parameter (GET /toy?exactPath=false) defined in the request, not sure if we can make it more flexible using OAS.

I have the feeling that OAS specifies exact matches. Especifically because you can define request and response bodies schemas. It does not make sense to specify request and response body for a "family" of paths. Also because it allows path templating.

However, to be flexible, I guess it is interesting to have some command flag to specify exact match VS prefix match. I am going to implement it in kuadrantctl generate istio virtualservice command.

Regarding The most specific match will take precedence, that statement is only for that example, not a general rule. If you reorder the rules like this:

apiVersion: gateway.networking.k8s.io/v1alpha2
kind: HTTPRoute
metadata:
  name: bar-route
spec:
  parentRefs:
  - name: example-gateway
  hostnames:
  - "bar.example.com"
  rules:
  - backendRefs:
    - name: bar-svc
      port: 8080
  - matches:
    - headers:
      - type: Exact
        name: env
        value: canary
    backendRefs:
    - name: bar-svc-canary
      port: 8080

My guess is that the selected backend will always be bar-svc and not the rule with the most specific match

Copy link
Contributor Author

@jjaferson jjaferson Jan 31, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the flexibility to specify the param pathprefix in the query string and to set the pathType to PathPrefix.

A request to GET /toy/something?pathprefix can fall back to GET /toy as the pathprefix param is defined and required

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO the path match type should be specified either by a) OpenAPI kuadrant custom extension or b) command option in the CLI.

The API spec should not change adding an "extra" query param to configure this behavior.

As said before, I am going to implement it in kuadrantctl generate istio virtualservice command with an optional flag (option b)

Copy link
Contributor Author

@jjaferson jjaferson Feb 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that I think about it, makes sense to define how the route will behave via the CLI, I'll make the change

Just to sumarize what you discussed in a call, we are going to create an OAS extension to allow the path matcher type be defined by the API spec, this will add more flexibility on how the traffic will be routed.

pkg/gatewayapi/http_route.go Outdated Show resolved Hide resolved
@eguzki
Copy link
Collaborator

eguzki commented Jan 28, 2022

Missing some (basic) doc at README.md and /doc/generate-gatewayapi-httproute.md

cmd/generate_gatewayapi.go Outdated Show resolved Hide resolved
@jjaferson jjaferson force-pushed the generation-gatewayapi branch 2 times, most recently from 72df1ed to 74180e6 Compare January 31, 2022 16:05
@jjaferson jjaferson force-pushed the generation-gatewayapi branch from 74180e6 to 8997999 Compare January 31, 2022 16:46
@jjaferson jjaferson force-pushed the generation-gatewayapi branch from 8997999 to 57f8323 Compare February 3, 2022 15:05
@jjaferson jjaferson merged commit 8ebb58b into main Feb 3, 2022
@eguzki eguzki deleted the generation-gatewayapi branch February 7, 2022 23:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants