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

Conditional "per_filter_config" #8659

Open
wbpcode opened this issue Oct 18, 2019 · 12 comments
Open

Conditional "per_filter_config" #8659

wbpcode opened this issue Oct 18, 2019 · 12 comments
Labels
design proposal Needs design doc/proposal before implementation help wanted Needs help!

Comments

@wbpcode
Copy link
Member

wbpcode commented Oct 18, 2019

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
Copy link
Contributor

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

@mattklein123 mattklein123 added the question Questions that are neither investigations, bugs, nor enhancements label Oct 18, 2019
@wbpcode
Copy link
Member Author

wbpcode commented Oct 21, 2019

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
Copy link
Member

@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
Copy link
Contributor

stevenzzzz commented Oct 21, 2019 via email

@wbpcode
Copy link
Member Author

wbpcode commented Oct 22, 2019

@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
Copy link
Member Author

wbpcode commented Oct 22, 2019

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
Copy link
Member

I think this is probably related to #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

@mattklein123 mattklein123 added design proposal Needs design doc/proposal before implementation help wanted Needs help! and removed question Questions that are neither investigations, bugs, nor enhancements labels Oct 22, 2019
@htuch
Copy link
Member

htuch commented Oct 23, 2019

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
Copy link
Member Author

wbpcode commented Oct 23, 2019

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
Copy link
Member

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
Copy link
Member

htuch commented Oct 24, 2019

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

@mattklein123
Copy link
Member

Another possible duplicate of #12968. cc @snowp @yangminzhu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design proposal Needs design doc/proposal before implementation help wanted Needs help!
Projects
None yet
Development

No branches or pull requests

4 participants