-
Notifications
You must be signed in to change notification settings - Fork 337
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
feat(*) expand Mesh resource to control passthrough #1058
Conversation
Signed-off-by: Nikolay Nikolaev <[email protected]>
Signed-off-by: Nikolay Nikolaev <[email protected]>
Should it be “true” by default instead? |
This definitely should be true by default. Otherwise we will be breaking current deployments when we deploy Kuma |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we maybe have E2E test for this? :)
Signed-off-by: Nikolay Nikolaev <[email protected]>
Signed-off-by: Nikolay Nikolaev <[email protected]>
Signed-off-by: Nikolay Nikolaev <[email protected]>
d639809
to
b597704
Compare
api/mesh/v1alpha1/mesh_helpers.go
Outdated
package v1alpha1 | ||
|
||
func (m *Mesh) IsPassthrough() bool { | ||
if m.GetNetworking() == nil || m.GetNetworking().GetOutbound() == nil || m.GetNetworking().GetOutbound().GetPassthrough() == nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: you can do just m.GetNetworking().GetOutbound().GetPassthrough() == nil
, check implementation of those getters, they handle nulls
_, stderr, err := cluster.ExecWithRetries(TestNamespace, clientPod.GetName(), "demo-client", | ||
"curl", "-v", "-m", "3", "--fail", "http://externalservice-http-server.externalservice-namespace") | ||
Expect(err).ToNot(HaveOccurred()) | ||
Expect(stderr).To(ContainSubstring("HTTP/1.1 200 OK")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
give some description of steps in this test please, example:
// given Mesh with passthrough false communication is not established
_, err := retry.DoWithRetryE
Expect(err).To(HaveOccurred())
// when applied Mesh with passthrough true
err = YamlK8s(fmt.Sprintf(meshDefaulMtlsOn, "true"))(cluster)
// then communication outside of the Mesh works
_, stderr, err := cluster.ExecWithRetries
Expect(err).ToNot(HaveOccurred())
Expect(stderr).To(ContainSubstring("HTTP/1.1 200 OK"))
}) | ||
Expect(err).To(HaveOccurred()) | ||
|
||
err = YamlK8s(fmt.Sprintf(meshDefaulMtlsOn, "true"))(cluster) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a note for whoever will take care of E2E tests eventually. We need proper builders for our entities. Something like this:
MeshBuilder().WithPassThrough(false).WithMTLS().KubeYAML()
Copy-pasting those YAMLs with parameters over many test is not the best idea IMHO.
Signed-off-by: Nikolay Nikolaev <[email protected]>
240d8ff
to
12985ef
Compare
Signed-off-by: Nikolay Nikolaev <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some typos, other than this 👍 I like that we are adding so much with so little of code :)
api/mesh/v1alpha1/mesh.proto
Outdated
google.protobuf.BoolValue passthrough = 1; | ||
} | ||
|
||
// Outbounf settings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Outbounf
-> Outbound
api/mesh/v1alpha1/mesh.proto
Outdated
@@ -70,6 +73,19 @@ message CertificateAuthorityBackend { | |||
google.protobuf.Struct conf = 4; | |||
} | |||
|
|||
// Networking defines he networking configuration of the mesh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
he
-> the
err = YamlK8s(fmt.Sprintf(meshDefaulMtlsOn, "false"))(cluster) | ||
Expect(err).ToNot(HaveOccurred()) | ||
|
||
// then accessing the external service is no logne possible |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
longne
-> longer
Signed-off-by: Nikolay Nikolaev <[email protected]>
Summary
Expanding the Mesh resource with pass-through control capabilities.
The setting defaults to
true
and needs to be explicitly set tofalse
to disallow access to non-mesh resources.Documentation