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

feat(gateway-api): Add custom backendRef and filters support for HTTPRoute #1742

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kahirokunn
Copy link
Contributor

@kahirokunn kahirokunn commented Dec 13, 2024

Description

This PR adds support for custom backend references in Flagger's primary and canary services. This enhancement allows users to specify different routing configurations and intermediate services for primary and canary traffic, enabling more complex deployment patterns and better integration with existing infrastructure.

Key Changes

  • Added backendRef and filters to spec.service.canary and spec.service.primary
  • Updated Gateway API router to support custom backend references
  • Modified service reconciliation logic to handle custom backend configurations
  • Added support for service-specific filters
  • Maintained backward compatibility with existing configurations

Use Cases

This feature enables several important scenarios:

  1. Routing through security proxies
  2. Adding service-specific monitoring
  3. Implementing different circuit breaker configurations
  4. Supporting complex mesh architectures
  5. Applying different filtering rules for primary and canary traffic

Example Configuration

apiVersion: flagger.app/v1beta1
kind: Canary
metadata:
  name: my-app
spec:
  service:
    primary:
      filters:
        - type: RequestHeaderModifier
          requestHeaderModifier:
            set:
              - name: x-route
                value: primary
    canary:
      backendRef:
        name: canary-proxy
        namespace: monitoring
        port: 3456
      filters:
        - type: RequestHeaderModifier
          requestHeaderModifier:
            set:
              - name: x-route
                value: canary

Breaking Changes

None. This is a backward-compatible change that maintains existing behavior when custom backend references are not specified.

Additional Context

This change also allows for cases where different backends are referenced for canary and primary, as shown in the attached image.
For details, please refer to the following PR.
#1714

Issue

#1741

TODO

  • add finalize ReferenceGrants

@kahirokunn kahirokunn changed the title feat: Custom BackendRef for Primary and Canary Services [WIP] feat: Custom BackendRef for Primary and Canary Services Dec 13, 2024
@kahirokunn kahirokunn marked this pull request as draft December 13, 2024 08:47
@kahirokunn kahirokunn force-pushed the custom-http-route branch 2 times, most recently from b711e6d to df378db Compare December 16, 2024 02:02
@codecov-commenter
Copy link

codecov-commenter commented Dec 16, 2024

Codecov Report

Attention: Patch coverage is 45.83333% with 195 lines in your changes missing coverage. Please review.

Project coverage is 39.34%. Comparing base (febc327) to head (cb59056).
Report is 16 commits behind head on main.

Files with missing lines Patch % Lines
...ped/gatewayapi/v1beta1/fake/fake_referencegrant.go 22.05% 52 Missing and 1 partial ⚠️
...g/apis/gatewayapi/v1beta1/zz_generated.deepcopy.go 41.37% 50 Missing and 1 partial ⚠️
pkg/apis/flagger/v1beta1/zz_generated.deepcopy.go 0.00% 24 Missing ⚠️
pkg/router/gateway_api.go 81.10% 19 Missing and 5 partials ⚠️
...ernalversions/gatewayapi/v1beta1/referencegrant.go 0.00% 21 Missing ⚠️
...rsioned/typed/gatewayapi/v1beta1/referencegrant.go 0.00% 9 Missing ⚠️
...lient/listers/gatewayapi/v1beta1/referencegrant.go 0.00% 4 Missing ⚠️
pkg/router/kubernetes_default.go 70.00% 2 Missing and 1 partial ⚠️
...oned/typed/gatewayapi/v1beta1/gatewayapi_client.go 0.00% 2 Missing ⚠️
...s/externalversions/gatewayapi/v1beta1/interface.go 0.00% 2 Missing ⚠️
... and 1 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1742      +/-   ##
==========================================
+ Coverage   39.27%   39.34%   +0.07%     
==========================================
  Files         284      288       +4     
  Lines       22379    22665     +286     
==========================================
+ Hits         8789     8918     +129     
- Misses      12643    12794     +151     
- Partials      947      953       +6     

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

@kahirokunn kahirokunn force-pushed the custom-http-route branch 7 times, most recently from f89feb6 to 5931ff6 Compare December 16, 2024 06:58
@kahirokunn kahirokunn changed the title [WIP] feat: Custom BackendRef for Primary and Canary Services [WIP] feat: Custom Backend for Primary and Canary Services Dec 16, 2024
@kahirokunn kahirokunn changed the title [WIP] feat: Custom Backend for Primary and Canary Services feat(gateway-api): Custom Backend for Primary and Canary Services Dec 16, 2024
@kahirokunn kahirokunn marked this pull request as ready for review December 17, 2024 04:27
@kahirokunn kahirokunn changed the title feat(gateway-api): Custom Backend for Primary and Canary Services feat(gateway-api): Add custom backendRef and filters support for HTTPRoute Dec 17, 2024
@kahirokunn
Copy link
Contributor Author

Dear @stefanprodan

I hope this message finds you well.

I am reaching out to request your review on an enhancement I am working on for the Flagger gateway API. My goal is to support the integration of Envoy Gateway with KEDA HTTPScaledObjects through this enhancement.

I would greatly appreciate your feedback and insights on this matter.

Thank you for your time and consideration.

Best regards,
kahirokunn

Signed-off-by: kahirokunn <[email protected]>
@aryan9600
Copy link
Member

thank you for this PR @kahirokunn! have you tested how this change behaves when performing a canary rollout with session affinity enabled? that code also makes use of backend specific filters, so its important to verify that any userland configuration will not break that feature.

CustomMetadata

// Ref references a Kubernetes object.
BackendObjectReference *v1.BackendObjectReference `json:"backendRef,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

lets avoid adding this in the same place as kubernetes service configuration (CustomMetadata). BackendObjectReference and Filters fall under a different abstraction.
my suggestion would be to add two fields to CanaryService, PrimaryHTTPBackendRef and CanaryHTTPBackendRef both of type v1.HTTPBackendRef.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your helpful suggestion. I will make the changes accordingly by introducing two new fields, PrimaryHTTPBackendRef and CanaryHTTPBackendRef, both of type v1.HTTPBackendRef, ensuring that they reside in the correct abstraction layer. Please let me know if there is anything else I can improve or clarify.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aryan9600 Thank you for the suggestion. However, when using v1.HTTPBackendRef, we are required to provide a name even if we only want to customize the filters, since the name field in BackendObjectReference is not optional. We feel this may not be the most optimal interface design, which is why we initially split backendRef and filters in our PR. What are your thoughts on addressing this concern?

@kahirokunn
Copy link
Contributor Author

Thank you so much for your feedback regarding session affinity! I will do my best to verify that these changes won’t break any existing session affinity behavior. However, to avoid any misunderstanding or missing test scenarios, would you mind sharing a bit more detail on the specific cases or concerns you have in mind about backend-specific filters and userland configurations? Your insights would be really helpful, and I appreciate your cooperation.

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