Design: External Processing support #5866
Conversation
Signed-off-by: gang.liu <[email protected]>
Signed-off-by: gang.liu <[email protected]>
izturn
requested review from
stevesloka and
sunjayBhatia
and removed request for
a team
izturn
added
kind/design
|
The Contour project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to the #contour channel in the Kubernetes Slack |
github-actions
bot
added
the
lifecycle/stale
izturn
removed
the
lifecycle/stale
|
The Contour project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to the #contour channel in the Kubernetes Slack |
github-actions
bot
added
the
lifecycle/stale
github-actions
bot
removed
the
lifecycle/stale
|
Sorry, I am very busy this month, But this task will be my main focus next month |
|
The Contour project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to the #contour channel in the Kubernetes Slack |
github-actions
bot
added
the
lifecycle/stale
izturn
removed
the
lifecycle/stale
|
ping @clayton-gonsalves |
| externalProcessing: | ||
| disabled: false | ||
| processor: | ||
| extensionRef: | ||
| apiVersion: projectcontour.io/v1alpha1 | ||
| name: extproc-extsvc3 | ||
| namespace: extproc-test | ||
| failOpen: true | ||
| responseTimeout: 31s | ||
| processingMode: | ||
| requestBodyMode: NONE | ||
| requestHeaderMode: SKIP | ||
| requestTrailerMode: SKIP | ||
| responseBodyMode: NONE | ||
| responseHeaderMode: SEND | ||
| responseTrailerMode: SKIP |
There was a problem hiding this comment.
In my opinion, repeating the whole ExternalProcessing struct at route level is not optimal API - it gets too verbose and I think for other similar cases we have used different approach earlier.
What would you think about (re)using the approach that JWT verification implements, where we define one or more named processors at virtualhost level and then at route level refer to one processor by its name? In that case, one or more routes can refer to a processor without repeating the full set of parameters at each route.
There was a problem hiding this comment.
I considered this approach, but did not adopt it for the following reasons:
-
The current design is based on the auth implementation, not the jwt.
-
From the Envoy configuration definition, the structure of jwt and ext_proc is very different. In jwt the external server(s) defined at the virtualhost level are only used as a temporary storage place for server(s) (and then referenced at the route level) and do not have a real effect. However, this is not the case with ext_proc. Therefore, if multiple server(s) are defined at the virtualhost level, then at each route level, the corresponding server(s) must be explicitly enabled or disabled, which increases the difficulty of configuration.
-
The jwt scheme is similar to the scheme we designed in the first version. In our actual use, we have found that while this is flexible, it is more difficult to understand and use.
Based on the above reasons and our internal practices, I personally believe that the current design is more reasonable.
There was a problem hiding this comment.
In jwt the external server(s) defined at the virtualhost level are only used as a temporary storage place for server(s) (and then referenced at the route level) and do not have a real effect. However, this is not the case with ext_proc. Therefore, if multiple server(s) are defined at the virtualhost level, then at each route level, the corresponding server(s) must be explicitly enabled or disabled, which increases the difficulty of configuration.
I might be missing something, since I do not see the difference:
The JWT API defines providers as temporary storage for server info as you say, but it also allows setting one as the default provider which is then automatically applied to all routes. Routes can still opt-out by disabling JWT verification explicitly, or require a specific (non-default) provider.
On the other hand, if I understood the current proposal correctly, for (non-default) route level external processor, the current API requires user to re-define the external processor server parameters again on each route, even in the case when two separate routes would share single processor. Wouldn't "distributing" the configuration like that lead into duplication of configuration?
There was a problem hiding this comment.
The current design prioritizes flexibility but results in excessive verbosity. It might be beneficial to introduce a vhost-level extproc setting to alleviate redundancy, especially within the same vhost/proxy.
The configuration on the config seems a bit in between to me where we provide flexibilty but in a sense limited. Historically, we've maintained global defaults in the configuration, managed by admins or cluster admins. IMO, service-specific settings within it feels somewhat out of place
|
The Contour project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to the #contour channel in the Kubernetes Slack |
github-actions
bot
added
the
lifecycle/stale
izturn
removed
the
lifecycle/stale
|
The Contour project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to the #contour channel in the Kubernetes Slack |
github-actions
bot
added
lifecycle/stale
|
The Contour project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to the #contour channel in the Kubernetes Slack |
github-actions
bot
added
the
lifecycle/stale
izturn
removed
the
lifecycle/stale
|
The Contour project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to the #contour channel in the Kubernetes Slack |
github-actions
bot
added
the
lifecycle/stale
izturn
removed
the
lifecycle/stale
|
The Contour project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to the #contour channel in the Kubernetes Slack |
github-actions
bot
added
lifecycle/stale
|
The Contour project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to the #contour channel in the Kubernetes Slack |
github-actions
bot
added
the
lifecycle/stale
izturn
removed
the
lifecycle/stale
|
The Contour project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to the #contour channel in the Kubernetes Slack |
github-actions
bot
added
lifecycle/stale
|
The Contour project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to the #contour channel in the Kubernetes Slack |
github-actions
bot
added
the
lifecycle/stale
github-actions
bot
removed
the
lifecycle/stale
|
The Contour project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to the #contour channel in the Kubernetes Slack |
github-actions
bot
added
the
lifecycle/stale
izturn
removed
the
lifecycle/stale
Signed-off-by: Gang Liu [email protected]
Design proposal to address #5123