config/meshConfig: New localProxyMode field #4686
Conversation
Add new `spec.sidecar.localProxyMode` field for user to control how mesh traffic gets proxied Signed-off-by: Keith Mattix II <keithmattix2@gmail.com>
keithmattix
requested review from
shashankram,
snehachhabria,
nojnhuh,
draychev,
jaellio and
trstringer
as code owners
pkg/configurator/methods.go
Outdated
| case configv1alpha2.LocalProxyModeLocalhost, configv1alpha2.LocalProxyModePodIP: | ||
| return proxyConfig | ||
| default: | ||
| // unrecognized proxy mode; return the default: ProxyModeLocalhost |
There was a problem hiding this comment.
Since the API rejects any other value up front anyway and the default is defined in the CRD, I have a feeling enforcing the default again here isn't worth it since any other theoretical implementation of this interface (including the mock used in tests) would presumably need to do the same thing which may not be obvious. Then we wouldn't necessarily need to add this method to the interface and GetMeshConfig().Spec.Sidecar.LocalProxyMode could be used directly instead.
There was a problem hiding this comment.
I just followed the approach envoyLogLevel did. I added the function GetLocalProxyMode because it allows us to explicitly define this behavior on the Configurator interface, rather than an implementation detail of MeshConfig. You're right though, at the moment, it's not likely this codepath gets hit
|
Do we want this to be a mesh-wide option? Are there situations where a user might want to use a mix (particularly if they use other OSS services outside of their control)? Would it make more sense for this to be a pod annotation? |
|
@steeling I think a mesh-wide approach is best for a few reasons:
|
Signed-off-by: Keith Mattix II <keithmattix2@gmail.com>
Codecov Report
@@ Coverage Diff @@
## main #4686 +/- ##
==========================================
- Coverage 69.44% 69.44% -0.01%
==========================================
Files 217 217
Lines 15287 15419 +132
==========================================
+ Hits 10616 10707 +91
- Misses 4621 4662 +41
Partials 50 50
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Leaving a note as a reminder to update the Mesh Configuration doc
Add new `spec.sidecar.localProxyMode` field for user to control how mesh traffic gets proxied Signed-off-by: Keith Mattix II <keithmattix2@gmail.com>
Add new
spec.sidecar.localProxyModefield for user tocontrol how mesh traffic gets proxied
Signed-off-by: Keith Mattix II keithmattix2@gmail.com
Description:
Part of the fix for #4653
Testing done:
Affected area:
Please answer the following questions with yes/no.
Does this change contain code from or inspired by another project? no
Is this a breaking change? no
Has documentation corresponding to this change been updated in the osm-docs repo (if applicable)?