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

fix(MeshHTTPRoute): allow "kuma.io/unresolved-backend" service name for GAMMA compliance #9670

Conversation

bartsmykla
Copy link
Contributor

Normally, service names cannot contain forward slashes ("/"). However, there are exceptions during resource conversion:

  • Gateway API to Kuma MeshHTTPRoute Conversion When converting an HTTPRoute from Gateway API to a MeshHTTPRoute (Kuma's resource definition), there might be situations where the targeted backend reference cannot be found. In such cases, Kuma sets the service name to "kuma.io/unresolved-backend". This name includes a forward slash, but it's allowed as an exception to handle unresolved references.

Checklist prior to review

  • Link to relevant issue as well as docs and UI issues
    • No relevant issues
  • This will not break child repos: it doesn't hardcode values (.e.g "kumahq" as a image registry) and it will work on Windows, system specific functions like syscall.Mkfifo have equivalent implementation on the other OS
    • It won't
  • Tests (Unit test, E2E tests, manual test on universal and k8s)
    • Manually tested
    • Don't forget ci/ labels to run additional/fewer tests
  • Do you need to update UPGRADE.md?
    • There is no need
  • Does it need to be backported according to the backporting policy? (this GH action will add "backport" label based on these file globs, if you want to prevent it from adding the "backport" label use no-backport-autolabel label)
    • There is no need

…name

Normally, service names cannot contain forward slashes ("/"). However,
there are exceptions during resource conversion:
* Gateway API to Kuma MeshHTTPRoute Conversion
  When converting an HTTPRoute from Gateway API to a MeshHTTPRoute
  (Kuma's resource definition), there might be situations where the
  targeted backend reference cannot be found. In such cases, Kuma sets
  the service name to "kuma.io/unresolved-backend". This name includes
  a forward slash, but it's allowed as an exception to handle unresolved
  references.

Signed-off-by: Bart Smykla <bartek@smykla.com>
@bartsmykla bartsmykla requested a review from a team as a code owner March 20, 2024 07:39
@bartsmykla bartsmykla requested review from jijiechen and Automaat and removed request for a team March 20, 2024 07:39
@lahabana
Copy link
Contributor

I think this value seems weird. Weakening validation seems pretty risky to me... what's @michaelbeaumont 's opinion?

@bartsmykla
Copy link
Contributor Author

bartsmykla commented Mar 20, 2024

@lahabana that's how it currently work for MeshGatewayRoute's (re. 97552c1) and according to code, that's how it should work for GAMMA (it doesn't because validation is not allowing to convert HTTPRoutes to MeshHTTPRoutes when targeted backend is not found)

What could be done is what @michaelbeaumont suggested here:

A different option would be to mark a backend explicitly as missing endpoints, as opposed to a special tag value:

backends:
  - unresolvedEndpoints: true
  - destination:
      kuma.io/service: some-working-backend

…ved-backend-service-name-in-gamma-httproute-conversion
…ved-backend-service-name-in-gamma-httproute-conversion
@michaelbeaumont
Copy link
Contributor

Yeah, the option as Bart pointed out would work. I wouldn't look at it as weakening validation per se, it's just to allow us to use a reserved string, same as we do for keys. Is there another way to prevent a collision with a real service?

@bartsmykla
Copy link
Contributor Author

We can switch this behavior in following up PR. I would vote for merging this as currently when HTTPRoute is targeting GAMMA resources with non-existing backendRefs, our validation webhook just refuses the conversion (which is incorrect).

@bartsmykla bartsmykla enabled auto-merge (squash) March 22, 2024 07:10
@bartsmykla bartsmykla merged commit fadac20 into kumahq:master Mar 22, 2024
15 checks passed
@bartsmykla bartsmykla deleted the fix/allow-unresolved-backend-service-name-in-gamma-httproute-conversion branch March 22, 2024 07:18
@lahabana lahabana changed the title fix(GAMMA/MeshHTTPRoute): allow "kuma.io/unresolved-backend" service name fix(MeshHTTPRoute): allow "kuma.io/unresolved-backend" service name for GAMMA compliance Apr 11, 2024
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.

None yet

3 participants