Kuadrant / kuadrantctl

Kuadrant configuration command line utility
Apache License 2.0
6 stars 13 forks source link

Copy labels #44

Closed jasonmadigan closed 9 months ago

jasonmadigan commented 9 months ago

Copy labels

It's kinda questionable how much we want to do here, but for a demonstration, I thought it would flow smoothly if the generated HTTPRoute included other things like rules and labels.

You could stretch this further to copy any arbitrary items from the resource - I'm not sure how far we should go.

Sample OAS spec (from api-poc-petstore)

---
openapi: 3.0.2
info:
  title: Swagger Petstore - OpenAPI 3.0
  version: 1.0.18
  x-kuadrant:
    route:
      name: "petstore"
      namespace: "petstore"
      labels:
        deployment: petstore
      hostnames:
        - petstore.stitch1.hcpapps.net
      parentRefs:
        - name: prod-web
          namespace: kuadrant-multi-cluster-gateways
          kind: Gateway
  description: |-
kuadrantctl generate gatewayapi httproute --oas src/main/resources/openapi.yaml | yq -P
kind: HTTPRoute
apiVersion: gateway.networking.k8s.io/v1beta1
metadata:
  name: petstore
  namespace: petstore
  creationTimestamp: null
  labels:
    deployment: petstore
spec:
  parentRefs:
    - kind: Gateway
      namespace: kuadrant-multi-cluster-gateways
      name: prod-web
  hostnames:
    - petstore.example.com
  rules:
    - backendRefs:
        - name: petstore
          port: 8080
    - matches:
        - path:
            type: PathPrefix
            value: /api/v3/user/login
          method: GET
      backendRefs:
        - name: petstore
          port: 8080
status:
  parents: null
codecov-commenter commented 9 months ago

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Comparison is base (0379791) 0.46% compared to head (84982bc) 0.46%.

Files Patch % Lines
cmd/generate_gatewayapi_httproute.go 0.00% 8 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## httproute-kuadrant-extensions #44 +/- ## ================================================================ - Coverage 0.46% 0.46% -0.01% ================================================================ Files 15 15 Lines 643 650 +7 ================================================================ Hits 3 3 - Misses 640 647 +7 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

jasonmadigan commented 9 months ago

Looking again.. after starting to now look at an RLP in an OAS spec.

for backendRefs, I think all we're missing is the port. so maybe this is even simpler. This can stay a WIP until I figure out what changes (if any) we actually need!

eguzki commented 9 months ago

That first "custom rule"

 rules:
        - backendRefs:
          - name: petstore
            port: 8080

is a catch all rule that will route all traffic to backend. Depending on the order that the catch-all rule lies in the final HTTPRoute, it could shadow all the remaining rules.

eguzki commented 9 months ago

Good contribution @jasonmadigan !!

I am in for the labels.

Not sure, tho, about the "custom rules". The rule, consisting of the triple {matches, filters, backendrefs} is indeed not fully covered from the OAS doc. Only matches can be inferred. Missing filters and backendrefs. There is an experimental property called timeouts that is also missing obviously. I was thinking about the filters. Something on the kuadrant extensions that allow adding filters to specific openapi path and/or operations. I decided to leave that out of scope for the demo, but definitely something to take into account for the future. If it is relevant, we can add that now.

Is there a use case to add custom "matchers" to the HTTPRoute? Something that OAS does not cover? Maybe something like "when there is a header X-CUSTOM == "something, route to this backend".

jasonmadigan commented 9 months ago

@eguzki updated so this just copies labels - removed the rules copying