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

CFP: HTTPRoute ExtensionRef support for arbitrary L7 Envoy filters #30587

Closed
GreenCappuccino opened this issue Feb 2, 2024 · 4 comments
Closed
Labels
area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. kind/feature This introduces new functionality. sig/agent Cilium agent related. stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale.

Comments

@GreenCappuccino
Copy link

GreenCappuccino commented Feb 2, 2024

Cilium Feature Proposal

Thanks for taking time to make a feature proposal for Cilium! If you have usage questions, please try the slack channel and see the FAQ first.

Is your proposed feature related to a problem?

Currently, trying to use an HTTPRoute in conjunction with additional Envoy filters (via CiliumEnvoyConfig) is difficult. This is a common use case when needing to provide authentication to a service like via the OAuth2 Envoy filter.

Describe the feature you'd like

I would like Cilium to add an EnvoyFilter resource for use as an HTTPRoute ExtensionRef. There is similar behavior present in the Envoy Gateway implementation of HTTPRoute, where it allows a reference to a CRD for each possible filter. This is a far more ergonomic way of defining envoy filters, and is a stable part of the Gateway API.

(Optional) Describe your proposed solution

Possibly create a new CRD, maybe CiliumEnvoyHTTPFilter, which can specify (multiple?) envoy filters, that can be the target of an HTTPRoute filter. The format can be similar to CiliumEnvoyConfig but scoped to Envoy HTTP filters. This filter will be merged into the filterchain during reconciliation of the HTTPRoute.

Example:

apiVersion: cilium.io/vX
kind: CiliumEnvoyHTTPFilter
metadata:
  name: oauth2-example
spec:
  - name: envoy.filters.http.oauth2
    typed_config:
      "@type": type.googleapis.com/envoy.extensions.filters.http.oauth2.v3.OAuth2
      config:
        token_endpoint:
          cluster: oauth
          uri: oauth.com/token
          timeout: 3s
        authorization_endpoint: https://oauth.com/oauth/authorize/
        redirect_uri: "%REQ(x-forwarded-proto)%://%REQ(:authority)%/callback"
        redirect_path_matcher:
          path:
            exact: /callback
        signout_path:
          path:
            exact: /signout
        credentials:
          client_id: foo
          token_secret:
            name: token
            sds_config:
              path: "/etc/envoy/token-secret.yaml"
          hmac_secret:
            name: hmac
            sds_config:
              path: "/etc/envoy/hmac.yaml"
        # (Optional): defaults to 'user' scope if not provided
        auth_scopes:
          - user
          - openid
          - email
        # (Optional): set resource parameter for Authorization request
        resources:
          - oauth2-resource
          - http://example.com
---
apiVersion: gateway.networking.k8s.io/v1beta1
kind: HTTPRoute
metadata:
  name: example
spec:
  parentRefs:
    - name: eg
  hostnames:
    - www.example.com
  rules:
    - matches:
        - path:
            type: PathPrefix
            value: /foo
      filters:
        - type: ExtensionRef
          extensionRef:
            group: cilium.io
            kind: CiliumEnvoyHTTPFilter
            name: oauth2-example
      backendRefs:
        - name: backend
          port: 3000

The ergonomics of this are much better, and is, from what I understand, what the ExtensionRef system was created for.

@GreenCappuccino GreenCappuccino added the kind/feature This introduces new functionality. label Feb 2, 2024
@GreenCappuccino
Copy link
Author

GreenCappuccino commented Feb 3, 2024

I've looked into implementing this feature, and from what I understand, the implementation of this requires some changes to the way that Cilium currently transforms HTTPRoutes.

Currently, for each gateway, Cilium creates two Envoy RouteConfiguration resources, for insecure and secure listeners. These RouteConfiguration resources store all the routes for that gateway. There are two filterChains, with filterChainMatch just based on whether TLS is enabled or not. Each chain has a HttpConnectionManager filter that manages the route for that security. http_filters can be chained into the http_connection_manager.

The Problem

If an Envoy HttpFilter resource is added to the http_filters of the HttpConnectionManager, it will affect all Routes on that HttpConnectionManager. This is not desirable and goes against the Gateway API specification.

Proposed Solution

Split up each transform for an HTTPRoute into its own RouteConfiguration and HttpConnectionManager. This allows assigning each HTTPRoute its own chain of the Envoy HttpFilter resources on its own HttpConnectionManager without interfering with other routes. This necessitates creating more complex filterChainMatch rules for HttpConnectionManager filters.

Disclaimer

This is just from my understanding of the situation. I may have misunderstood something major. I also haven't looked too deeply into how Envoy Gateway implements this, which may be a helpful resource.

@lmb lmb added area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. sig/agent Cilium agent related. labels Feb 7, 2024
@youngnick
Copy link
Contributor

Thanks for this issue @GreenCappuccino, I really appreciate the time and effort you've put into describing the change.

You're right that extending Gateway API is what ExtensionRef was created for, but I think that we need to keep extensions we support tightly scoped and single-use, rather than adding a more broad filter that could be used for any Envoy filter.

John Howard from Istio put it like this on his blog about Istio's EnvoyFilter support:

EnvoyFilter is, objectively, the worst feature in Istio for stability. Essentially, it gives arbitrary patching into Envoy code. An analogy would be to provide a fast-moving project a git diff that is patched dynamically and recompiled; EnvoyFilter is only slightly more stable than that. In addition to risks of breakage, particularly around upgrades, safe usage requires a deep understanding of Envoy, which is surprisingly hard.
That being said, EnvoyFilter brings great power along with it. I would urge you to resist the temptation to use it as much as reasonably possible.

I understand that in this case, we're talking about a slightly more scoped resource that will allow injecting specific filters into the filter chain, but this still poses one main problem: we need to be able to pick where in the chain we put this filter. Too early, and you may be able to override the effect of other filters in the chain (we certainly don't want anyone overriding policy enforcement, for example!), too late, and some things (like auth) may not work reliably. This becomes more complex the more features we add using dedicated configuration (like Envoy Gateway's AuthenticationFilter for example, or some config for Rate limiting - the ordering behavior for the list of filters in a HTTPRoute is currently unspecified, so we can't necessarily just respect the order given in the HTTPRoute.)

I understand the frustration with the fact that all this technology is present right there in Envoy and you can't use it, but the problem is that once we add this, it's there forever. Even after we add better ways to do this by describing the same functionality in a more declarative way, we'll be stuck with supporting the ability for people to configure arbitrary Envoy filters.

I'm very interested to know what filters you're interested in seeing - noting that we already have a request for auth (#23797) and we know that rate limiting and circuit breaking are widely requested features.

As a maintainer for Gateway API as well, believe me when I say I really want everyone to be able to use all the amazing work that's built into Envoy - but I also want to make sure we add it in a way that minimizes the footgun effect and maximizes supportability.

So, what to do about this request? Well, as I said, I'm interested in talking more about what you'd like to see. As I said, I'm reluctant to add arbitrary filter support, but I'm interested in talking more about your requirements. Reluctant doesn't mean "in no possible universe", but it does mean it's a high bar.

Copy link

This issue has been automatically marked as stale because it has not
had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Apr 14, 2024
Copy link

This issue has not seen any activity since it was marked stale.
Closing.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. kind/feature This introduces new functionality. sig/agent Cilium agent related. stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale.
Projects
None yet
Development

No branches or pull requests

3 participants