envoyproxy / envoy

Cloud-native high-performance edge/middle/service proxy
https://www.envoyproxy.io
Apache License 2.0
24.91k stars 4.79k forks source link

Conditional "per_filter_config" #8659

Open wbpcode opened 5 years ago

wbpcode commented 5 years ago

In a multi-tenant scenario, for the same route, the same filter, different tenants may need to use different route-level filter configurations. Although #4704 can well classify route configurations based on HTTP attributes, it is too complex and heavy for scenarios where only filter configuration is different.

In addition, consider another scenario. Assume we have ten filters and two filters (filters A and B) of them that need to use different route-level config depending on the HTTP attributes. If we don't want to modify the code of filters A and B, then we have to create a lot of match&route to cover all cases. Although the other eight filters except A and B don't have any special configuration, their configuration must be copied.

If the filter can be configured with different route-level configurations depending on the HTTP attributes, then the above two problems can be solved better.

stevenzzzz commented 5 years ago

could you provide an example for better understanding why the SRDS+per_filterconfigs doesn't work in your multi-tenants scenario?

wbpcode commented 5 years ago

In my work scenario, there may be hundreds of different tenants. Most tenants share most of the configuration, but for a small number of plugins, each tenant needs a different configuration. Using SRDS can of course meet my needs, but I think SRDS is too complex for me.

I don't need tenant isolation at the 'virtualhosts' level like SRDS, I just need to provide different plugin configurations for different tenants in same route. Of course, when the number of tenants is small, there is no problem with using SRDS. At the time, when the number of tenants was large, SRDS generated a large number of redundant configurations in my usage scenario.

Below is a simple example. Suppose there are two tenants user1 and user2. Both tenants use Filter envoy.A and Filter envoy.B, where the configuration of envoy.A will vary from tenant to tenant, and the configuration of envoy.B is the same in both tenants.

With SRDS, the configuration is as follows. As you can see, although almost all configurations are the same, in order to isolate user1 and user2, SRDS must create two different virtualhosts. Except for the configuration of envoy.A, the two virtualhosts are identical. Now, in the case of two tenants, if there are 20 tenants or even 200 tenants?

scoped_route_config:
  name: scoped_routes             
  scope_key_builder:
    fragments:
    - header_element:
      name: User-Info
      element_separator: ,
      element:
        separator: =
        key: User-Name                                             
  scopes:
  - route_configuration_name: route_scope1
    key:
      fragments:
      - string_key: user1
  - route_configuration_name: route_scope2
    key:
      fragments:
      - string_key: user2

route_config:                                                             
- name: route_scope1                                                     
  virtual_hosts:                                                         
  - name: exapmle1                                              
    domains:                        
    - "*.example.com 
    routes:
    - match: { prefix: "/api/exemple" }                                         
      route: { cluster: cluster_example}
      per_filter_config:
        envoy.A:
          config: "config for user1"
        envoy.B:
          config: "config for all users" 
- name: route_scope2                                                     
  virtual_hosts:                                                         
  - name: exapmle1                                              
    domains:                        
    - "*.example.com 
    routes:
    - match: { prefix: "/api/exemple" }                                         
      route: { cluster: cluster_example}
      per_filter_config:
        envoy.A:
          config: "config for user2"
        envoy.B:
          config: "config for all users"

The configuration I actually hope is as follows.

route_config:                                       
  virtual_hosts:                                                         
  - name: exapmle1                                              
    domains:                        
    - "*.example.com 
    routes:
    - match: { prefix: "/api/exemple" }                                             
      route: { cluster: cluster_example}
      per_filter_config:
        envoy.A:
          case1:
            config: "config for user1"
          case2:
            config: "config for user2"
        envoy.B:
          config: "config for all users" 

This is why I want envoy to support conditional "per_filter_config". This feature is not contradictory to SRDS, but can be used as an extension of SRDS to make SRDS more flexible. Use SRDS in cases where some configurations require complete isolation. Use conditional "per_filter_config" in cases where you only need to customize different filter configurations for different tenants. @stevenzzzz @mattklein123

mattklein123 commented 5 years ago

@wbpcode in your optimal example above, how do you expect to select between case1 and case2? Some type of matching construct within the per_filter_config itself?

stevenzzzz commented 5 years ago

@wbpcode, in your example, is there any reason not using a dict as the per_filter_config for filter.A? In your filter code, I think you can again match based on some header value to get the config out of the dictionary?

On Mon, Oct 21, 2019 at 2:37 PM Matt Klein notifications@github.com wrote:

@wbpcode https://github.com/wbpcode in your optimal example above, how do you expect to select between case1 and case2? Some type of matching construct within the per_filter_config itself?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/envoyproxy/envoy/issues/8659?email_source=notifications&email_token=AA2I6TDOJYWPQGHPECOUL5LQPXZFNA5CNFSM4JCFOEGKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEB3K7EQ#issuecomment-544649106, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA2I6TFWKRNHNWZV2ESF72DQPXZFNANCNFSM4JCFOEGA .

-- Xin Zhuang

wbpcode commented 5 years ago

@wbpcode in your optimal example above, how do you expect to select between case1 and case2? Some type of matching construct within the per_filter_config itself?

Yep. A Header matcher or querystring matcher help filter to get expected config.

wbpcode commented 5 years ago

In fact, I have used the approach you said to solve the problem. However, there are two problems with this approach. First, existing official filters must be modified. Second, the matching logic must be embedded in the filters itself in the form of code, which is not flexible enough. Each time the matching logic changes, the filter must also be modified accordingly. @stevenzzzz

mattklein123 commented 5 years ago

I think this is probably related to https://github.com/envoyproxy/envoy/issues/8669 so maybe we can solve both issues at the same time. I think it's reasonable to have the ability to use a generic set of matchers to select the route config, disable a filter, etc. I'm going to mark this pending a design proposal and help wanted. cc @htuch

htuch commented 5 years ago

I think if we figure out what the real story is going to be around generic HTTP matchers, then we can just reuse the HTTP matcher definitions we have at per-route level to select the route rule within the per filter route config to further specialize.

One thing I want to be cautious about is over optimizing here if we aren't going to see a huge pay-off in terms of reduced configuration. By time we start expressing potentially complex matchers at the per-filter config level for each tenant, config sizes might grow. From a complexity perspective, let's say we have M routes and N tenants. The number of entries in the RouteConfiguraiton will be O(M). If we have N * O(M) RouteConfigurations with SRDS vs. O(NM) for sophisticated per-filter config matchers, the space complexity is the same. So, from a scalability perspective, I don't think we win, we just save some constant overhead (which may be appreciable).

wbpcode commented 5 years ago

Yes. When each tenant's configuration is completely different, conditional "per_filter_config" does not save much overhead compared to SRDS. Their complexity is similar (Only the constant terms are different). However, conditional "per_filter_config" can effectively reduce redundant configurations when there is only a small difference in the configurations of different tenants.

The effect of conditional "per_filter_config" depends on the actual work scenario. However, I still think this feature is necessary as a supplement of SRDS. The user can decide how to use it according to their requirements and scenario. @htuch

mattklein123 commented 5 years ago

Assuming generic matchers as @htuch mentioned, I think this is reasonable to implement and like I said could also likely cover the disable case. If someone wants to work on this lets get a design proposal going.

htuch commented 5 years ago

@wbpcode ack. I'm down with the plan that @mattklein123 outlines ^^.

mattklein123 commented 4 years ago

Another possible duplicate of https://github.com/envoyproxy/envoy/issues/12968. cc @snowp @yangminzhu