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

Copy labels #44

Merged
merged 1 commit into from
Nov 22, 2023
Merged

Conversation

jasonmadigan
Copy link
Member

@jasonmadigan jasonmadigan commented Nov 15, 2023

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

codecov-commenter commented Nov 15, 2023

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 ⚠️
Additional details and impacted files
@@                       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     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jasonmadigan jasonmadigan changed the title WIP [WIP] copy labels and rules Nov 15, 2023
@jasonmadigan
Copy link
Member Author

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!

@jasonmadigan jasonmadigan force-pushed the copy-labels-and-routes branch from db3dd9f to 08005b2 Compare November 15, 2023 14:33
@eguzki
Copy link
Collaborator

eguzki commented Nov 15, 2023

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

eguzki commented Nov 15, 2023

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 jasonmadigan changed the title [WIP] copy labels and rules Copy labels Nov 22, 2023
@jasonmadigan jasonmadigan force-pushed the copy-labels-and-routes branch from 08005b2 to 84982bc Compare November 22, 2023 10:24
@jasonmadigan
Copy link
Member Author

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

@eguzki eguzki merged commit ffb7f05 into httproute-kuadrant-extensions Nov 22, 2023
5 checks passed
@eguzki eguzki deleted the copy-labels-and-routes branch November 22, 2023 10:29
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.

3 participants