feat(kuma-cp) permissive mTLS mode #2579
Conversation
Signed-off-by: Ilya Lobkov <lobkovilya@yandex.ru>
Signed-off-by: Ilya Lobkov <lobkovilya@yandex.ru>
Signed-off-by: Ilya Lobkov <lobkovilya@yandex.ru>
|
|
||
| getServiceEndpoint := func() string { | ||
| var addr string | ||
| r := regexp.MustCompile(`::([0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}):?([0-9]{1,5})?::`) |
There was a problem hiding this comment.
how about kumactl get dataplanes and get address + port?
There was a problem hiding this comment.
yes, probably it's simpler, but at that moment I don't see the point in rewriting this to kumactl get dataplanes, the same amount of work, still have to parse the output somehow
|
|
||
| checkInsideMeshConnection() | ||
|
|
||
| checkNoOutsideMeshConnection() |
There was a problem hiding this comment.
Disclaimer: My opinion, your opinion may vary.
Please try to not overuse functions inside a test. For me, it's easier to read a test with code embedded
// given a mesh
... code that creates a mesh
// and test server
... code that creates test server
than test with functions. To read the full test I need to jump back and forth between test and function implementations.
Of course, if we have reusable functions and repeating the complex operations, it may make sense to use it.
Also, I'd try to keep the given, when, then convention. For me, it makes it easier to read the intention of the test.
There was a problem hiding this comment.
I agree that it was probably too much. I got rid of these checkInsideMeshConnection and checkNoOutsideMeshConnection, but I left the functions that create a mesh and runs client, they are 100 percent reusable across tests in this file
|
#2550 (comment) <- supporting this use case is critical for this functionality. |
Signed-off-by: Ilya Lobkov <lobkovilya@yandex.ru>
Codecov Report
@@ Coverage Diff @@
## master #2579 +/- ##
==========================================
- Coverage 52.08% 51.98% -0.11%
==========================================
Files 877 878 +1
Lines 49478 49628 +150
==========================================
+ Hits 25771 25799 +28
- Misses 21641 21759 +118
- Partials 2066 2070 +4
Continue to review full report at Codecov.
|
|
I added support of the case when the client and server already have TLS, covered this with a test. I'm going to provide docs on how to migrate your services under the mesh. Also, this PR probably requires a disclaimer for UPGRADE.md – you can't set PERMISSIVE mode until all Kuma CP instances are updated to the newer version, because ALPN protocol |
| @@ -14,6 +14,7 @@ import ( | |||
| envoy_names "github.com/kumahq/kuma/pkg/xds/envoy/names" | |||
| v3 "github.com/kumahq/kuma/pkg/xds/envoy/routes/v3" | |||
| "github.com/kumahq/kuma/pkg/xds/envoy/tags" | |||
| xds_tls "github.com/kumahq/kuma/pkg/xds/envoy/tls/v3" | |||
There was a problem hiding this comment.
we should not import individual Envoy versions in generator. KumaALPNProtocols should be common to all versions. I'd suggest moving KumaALPNProtocols out of pkg/xds/envoy/tls/v3 into pkg/xds/envoy/tls
Signed-off-by: Ilya Lobkov <lobkovilya@yandex.ru>
Signed-off-by: Ilya Lobkov <lobkovilya@yandex.ru> # Conflicts: # test/server/cmd/echo.go
Signed-off-by: Ilya Lobkov <lobkovilya@yandex.ru>
Summary
The initial implementation of the permissive mTLS model according to #2550
Full changelog
modeoption for MeshIssues resolved
N/A
Documentation
Testing
Backwards compatibility
backport-to-stablelabel if the code is backwards compatible. Otherwise, list breaking changes.