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

Remove code TODO for "support all standard HTTPRouteRule Filters" #2793

Open
1 of 2 tasks
mlavacca opened this issue Aug 9, 2022 · 3 comments
Open
1 of 2 tasks

Remove code TODO for "support all standard HTTPRouteRule Filters" #2793

mlavacca opened this issue Aug 9, 2022 · 3 comments
Labels
area/debt area/gateway-api Relating to upstream Kubernetes SIG Networking Gateway API

Comments

@mlavacca
Copy link
Member

mlavacca commented Aug 9, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Problem Statement

In the context of the conformance tests, #2776 has introduced support for HTTPRouteRule.Filters of type RequestHeaderModifier. This has been achieved using a request-modifier kong plugin. Since RequestHeaderModifier is only one among the possible standard HTTPRouteRule.Filters, we should support all of them.

Proposed Solution

Implementation of filters handled in separate issues

This issue is only to remove the code TODO as filter implementation is already accounted for on our roadmap.

Additional information

No response

Acceptance Criteria

  • remove the TODO in the code
@mlavacca mlavacca added area/feature New feature or request area/gateway-api Relating to upstream Kubernetes SIG Networking Gateway API labels Aug 9, 2022
@mflendrich
Copy link
Contributor

mflendrich commented Aug 9, 2022

@mlavacca, could you please split the core and extended filters into 2 separate issues, and add the core ones to Milestone 3 and the rest to Milestone 4? thx

RequestRedirect is currently not well understood so it might be worth labeling a spike.

@rainest
Copy link
Contributor

rainest commented Aug 9, 2022

For RequestRedirect, Kong routes do not have native configuration that will issue a redirect response. We furthermore do not have any plugin tailored for this purpose. There are two options to handle this currently:

  • Use the serverless plugin, which effectively allows you to write your own plugin as plugin configuration instead of files on the image filesystem. IMO we should avoid this because we recommend against enabling this plugin for security reasons.
  • Use a combination of request-termination (to issue a response with a chosen status) and response-transformer (to set a Location header). This would not support dynamic redirects that use part of the original request in the redirect location.

Any code we write for the serverless option is effectively plugin code. Our eventual goal should be to have a plugin in Kong's default plugin set that can handle dynamic redirects (replacing only part of the request as described by https://gateway-api.sigs.k8s.io/guides/http-redirect-rewrite/#redirects).

@shaneutt
Copy link
Contributor

shaneutt commented Aug 9, 2022

Given the note that the serverless plugin is a security risk, it would seem that is an option we shouldn't take at all. It would seem to me that if the second option is work-able for the current core requirements and conformance than we should start there and follow up with the "full" option described at the end.

@mlavacca mlavacca removed their assignment Aug 10, 2022
@mflendrich mflendrich changed the title Support all standard HTTPRouteRule Filters Remove code TODO for "support all standard HTTPRouteRule Filters" Oct 11, 2022
@mflendrich mflendrich added area/debt and removed area/feature New feature or request labels Oct 11, 2022
@programmer04 programmer04 removed this from the Gateway API - Next milestone Sep 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/debt area/gateway-api Relating to upstream Kubernetes SIG Networking Gateway API
Projects
None yet
Development

No branches or pull requests

5 participants