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

feat(*) expand Mesh resource to control passthrough #1058

Merged
merged 13 commits into from
Oct 14, 2020

Conversation

nickolaev
Copy link
Contributor

@nickolaev nickolaev commented Oct 8, 2020

Summary

Expanding the Mesh resource with pass-through control capabilities.

apiVersion: kuma.io/v1alpha1
kind: Mesh
metadata:
  name: default
spec:
  networking
    outbound:
      passthrough: false

The setting defaults to true and needs to be explicitly set to false to disallow access to non-mesh resources.

Documentation

Nikolay Nikolaev added 2 commits October 8, 2020 18:08
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
@nickolaev nickolaev requested a review from a team as a code owner October 8, 2020 15:17
@subnetmarco
Copy link
Contributor

Should it be “true” by default instead?

@jakubdyszkiewicz
Copy link
Contributor

This definitely should be true by default. Otherwise we will be breaking current deployments when we deploy Kuma

Copy link
Contributor

@jakubdyszkiewicz jakubdyszkiewicz left a 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? :)

pkg/xds/generator/transparent_proxy_generator.go Outdated Show resolved Hide resolved
Nikolay Nikolaev added 2 commits October 8, 2020 19:58
Nikolay Nikolaev added 2 commits October 9, 2020 19:04
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
package v1alpha1

func (m *Mesh) IsPassthrough() bool {
if m.GetNetworking() == nil || m.GetNetworking().GetOutbound() == nil || m.GetNetworking().GetOutbound().GetPassthrough() == nil {
Copy link
Contributor

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

test/e2e/externalservices_kubernetes_test.go Show resolved Hide resolved
_, 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"))
Copy link
Contributor

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

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.

test/e2e/externalservices_kubernetes_test.go Show resolved Hide resolved
Nikolay Nikolaev added 2 commits October 12, 2020 16:12
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
Nikolay Nikolaev added 2 commits October 12, 2020 19:49
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
Copy link
Contributor

@jakubdyszkiewicz jakubdyszkiewicz left a 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 :)

google.protobuf.BoolValue passthrough = 1;
}

// Outbounf settings
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Outbounf -> Outbound

@@ -70,6 +73,19 @@ message CertificateAuthorityBackend {
google.protobuf.Struct conf = 4;
}

// Networking defines he networking configuration of the mesh
Copy link
Contributor

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

longne -> longer

Nikolay Nikolaev added 2 commits October 13, 2020 16:09
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
@nickolaev nickolaev merged commit 95c7e99 into master Oct 14, 2020
@nickolaev nickolaev deleted the feat/disable_passthrough branch October 14, 2020 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants