Repository: kumahq/kuma
Pull request #2402 (2021-07-20 21:25:55)

chore(*) update remaining listener filter helpers (backport #2393)
This is an automatic backport of pull request #2393 done by [Mergify](https://mergify.io).


---


<details>
<summary>Mergify commands and options</summary>

<br />

More conditions and actions can be found in the [documentation](https://docs.mergify.io/).

You can also trigger Mergify actions by commenting on this pull request:

- `@Mergifyio refresh` will re-evaluate the rules
- `@Mergifyio rebase` will rebase this PR on its base branch
- `@Mergifyio update` will merge the base branch into this PR
- `@Mergifyio backport <destination>` will backport this PR on `<destination>` branch

Additionally, on Mergify [dashboard](https://dashboard.mergify.io/) you can:

- look at your merge queues
- generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.io/
</details>



Patch:
diff --git a/api/mesh/v1alpha1/dataplane_helpers.go b/api/mesh/v1alpha1/dataplane_helpers.go
index 633f04353..8fa86e50d 100644
--- a/api/mesh/v1alpha1/dataplane_helpers.go
+++ b/api/mesh/v1alpha1/dataplane_helpers.go
@@ -39,27 +39,16 @@ type ProxyType string
 const (
 	DataplaneProxyType ProxyType = "dataplane"
 	IngressProxyType   ProxyType = "ingress"
-	GatewayProxyType   ProxyType = "gateway"
 )
 
 func (t ProxyType) IsValid() error {
 	switch t {
-	case DataplaneProxyType, IngressProxyType, GatewayProxyType:
+	case DataplaneProxyType, IngressProxyType:
 		return nil
 	}
 	return errors.Errorf("%s is not a valid proxy type", t)
 }
 
-func (d *Dataplane) ProxyType() ProxyType {
-	if d.IsIngress() {
-		return IngressProxyType
-	}
-	if d.GetNetworking().GetGateway() != nil {
-		return GatewayProxyType
-	}
-	return DataplaneProxyType
-}
-
 type InboundInterface struct {
 	DataplaneAdvertisedIP string
 	DataplaneIP           string
@@ -402,15 +391,29 @@ func (d *Dataplane) GetIdentifyingService() string {
 	return ServiceUnknown
 }
 
+// IsIngress returns true if this Dataplane specifies an ingress
+// configuration.
+//
+// Deprecated in favor of ZoneIngress.
 func (d *Dataplane) IsIngress() bool {
 	if d.GetNetworking() == nil {
 		return false
 	}
-	return d.Networking.Ingress != nil
+	return d.GetNetworking().GetIngress() != nil
+}
+
+// IsDataplane returns true if this Dataplane specifies a gateway
+// configuration.
+func (d *Dataplane) IsGateway() bool {
+	if d.GetNetworking() == nil {
+		return false
+	}
+
+	return d.GetNetworking().GetGateway() != nil
 }
 
 func (d *Dataplane) HasPublicAddress() bool {
-	if d.Networking.Ingress == nil {
+	if !d.IsIngress() {
 		return false
 	}
 	return d.Networking.Ingress.PublicAddress != "" && d.Networking.Ingress.PublicPort != 0
diff --git a/app/kuma-dp/cmd/run.go b/app/kuma-dp/cmd/run.go
index 53ac26448..d0c430152 100644
--- a/app/kuma-dp/cmd/run.go
+++ b/app/kuma-dp/cmd/run.go
@@ -57,18 +57,19 @@ func newRunCmd(rootCtx *RootContext) *cobra.Command {
 			}
 
 			var err error
-			if mesh_proto.ProxyType(cfg.Dataplane.ProxyType) == mesh_proto.IngressProxyType {
+			switch mesh_proto.ProxyType(cfg.Dataplane.ProxyType) {
+			case mesh_proto.IngressProxyType:
 				proxyResource, err = readZoneIngressResource(cmd, cfg)
-				if err != nil {
-					runLog.Error(err, "unable to read provided zone ingress")
-					return err
-				}
-			} else {
+			case mesh_proto.DataplaneProxyType:
 				proxyResource, err = readDataplaneResource(cmd, cfg)
-				if err != nil {
-					runLog.Error(err, "unable to read provided dataplane")
-					return err
-				}
+			default:
+				return errors.Errorf("invalid proxy type %q (must be either %q or %q)",
+					cfg.Dataplane.ProxyType, mesh_proto.DataplaneProxyType, mesh_proto.IngressProxyType)
+			}
+
+			if err != nil {
+				runLog.Error(err, "unable to read provided %s policy", cfg.Dataplane.ProxyType)
+				return err
 			}
 
 			if proxyResource != nil {
diff --git a/pkg/config/app/kuma-dp/config.go b/pkg/config/app/kuma-dp/config.go
index 591ae9787..57e441d4d 100644
--- a/pkg/config/app/kuma-dp/config.go
+++ b/pkg/config/app/kuma-dp/config.go
@@ -8,7 +8,6 @@ import (
 	"go.uber.org/multierr"
 
 	mesh_proto "github.com/kumahq/kuma/api/mesh/v1alpha1"
-
 	"github.com/kumahq/kuma/pkg/config"
 	config_types "github.com/kumahq/kuma/pkg/config/types"
 )
@@ -188,19 +187,30 @@ func (d *Dataplane) Sanitize() {
 
 func (d *Dataplane) Validate() (errs error) {
 	proxyType := mesh_proto.ProxyType(d.ProxyType)
-	if err := proxyType.IsValid(); err != nil {
-		errs = multierr.Append(errs, errors.Wrap(err, ".ProxyType is not valid"))
+	switch proxyType {
+	case mesh_proto.DataplaneProxyType, mesh_proto.IngressProxyType:
+	default:
+		if err := proxyType.IsValid(); err != nil {
+			errs = multierr.Append(errs, errors.Wrap(err, ".ProxyType is not valid"))
+		} else {
+			// Not all Dataplane types are allowed to be set directly in config.
+			errs = multierr.Append(errs, errors.Errorf(".ProxyType %q is not supported", proxyType))
+		}
 	}
+
 	if d.Mesh == "" && proxyType != mesh_proto.IngressProxyType {
 		errs = multierr.Append(errs, errors.Errorf(".Mesh must be non-empty"))
 	}
+
 	if d.Name == "" {
 		errs = multierr.Append(errs, errors.Errorf(".Name must be non-empty"))
 	}
+
 	// Notice that d.AdminPort is always valid by design of PortRange
 	if d.DrainTime <= 0 {
 		errs = multierr.Append(errs, errors.Errorf(".DrainTime must be positive"))
 	}
+
 	return
 }
 
diff --git a/pkg/core/resources/apis/mesh/dataplane_overview_helpers.go b/pkg/core/resources/apis/mesh/dataplane_overview_helpers.go
index 15235181c..4fc33ba41 100644
--- a/pkg/core/resources/apis/mesh/dataplane_overview_helpers.go
+++ b/pkg/core/resources/apis/mesh/dataplane_overview_helpers.go
@@ -31,7 +31,7 @@ func (t *DataplaneOverviewResource) GetStatus() (Status, []string) {
 
 	allInboundsOffline := len(errs) == len(t.Spec.Dataplane.Networking.Inbound)
 	allInboundsOnline := len(errs) == 0
-	if t.Spec.Dataplane.ProxyType() == mesh_proto.GatewayProxyType {
+	if t.Spec.Dataplane.IsGateway() {
 		allInboundsOffline = false
 		allInboundsOnline = true
 	}
diff --git a/pkg/kds/util/resource.go b/pkg/kds/util/resource.go
index 799fc6b18..e0359abbd 100644
--- a/pkg/kds/util/resource.go
+++ b/pkg/kds/util/resource.go
@@ -66,8 +66,8 @@ func AddSuffixToNames(rs []model.Resource, suffix string) {
 
 func ZoneTag(r model.Resource) string {
 	dp := r.GetSpec().(*mesh_proto.Dataplane)
-	if dp.Networking.GetGateway() != nil {
-		return dp.Networking.GetGateway().GetTags()[mesh_proto.ZoneTag]
+	if dp.IsGateway() {
+		return dp.GetNetworking().GetGateway().GetTags()[mesh_proto.ZoneTag]
 	}
 	return dp.GetNetworking().GetInbound()[0].GetTags()[mesh_proto.ZoneTag]
 }
diff --git a/pkg/xds/bootstrap/generator.go b/pkg/xds/bootstrap/generator.go
index 6e4ecea53..1657ec9b8 100644
--- a/pkg/xds/bootstrap/generator.go
+++ b/pkg/xds/bootstrap/generator.go
@@ -95,7 +95,7 @@ func (b *bootstrapGenerator) Generate(ctx context.Context, request types.Bootstr
 			return nil, "", err
 		}
 		return b.generateFor(*proxyId, request, "ingress", adminPort)
-	case mesh_proto.DataplaneProxyType, mesh_proto.GatewayProxyType:
+	case mesh_proto.DataplaneProxyType:
 		proxyId := core_xds.BuildProxyId(request.Mesh, request.Name)
 		dataplane, err := b.dataplaneFor(ctx, request, proxyId)
 		if err != nil {
diff --git a/pkg/xds/envoy/listeners/configurers.go b/pkg/xds/envoy/listeners/configurers.go
index 0bea07ff0..f8191f7ff 100644
--- a/pkg/xds/envoy/listeners/configurers.go
+++ b/pkg/xds/envoy/listeners/configurers.go
@@ -1,6 +1,9 @@
 package listeners
 
 import (
+	envoy_listener "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3"
+	"google.golang.org/protobuf/types/known/wrapperspb"
+
 	mesh_proto "github.com/kumahq/kuma/api/mesh/v1alpha1"
 	mesh_core "github.com/kumahq/kuma/pkg/core/resources/apis/mesh"
 	core_xds "github.com/kumahq/kuma/pkg/core/xds"
@@ -204,10 +207,8 @@ func HttpStaticRoute(builder *envoy_routes.RouteConfigurationBuilder) FilterChai
 // HttpDynamicRoute configures the listener filter chain to dynamically request
 // the named RouteConfiguration.
 func HttpDynamicRoute(name string) FilterChainBuilderOpt {
-	return FilterChainBuilderOptFunc(func(config *FilterChainBuilderConfig) {
-		config.AddV3(&v3.HttpDynamicRouteConfigurer{
-			RouteName: name,
-		})
+	return AddFilterChainConfigurer(&v3.HttpDynamicRouteConfigurer{
+		RouteName: name,
 	})
 }
 
@@ -269,3 +270,25 @@ func DNS(vips map[string]string, emptyDnsPort uint32) ListenerBuilderOpt {
 		EmptyDNSPort: emptyDnsPort,
 	})
 }
+
+func ConnectionBufferLimit(bytes uint32) ListenerBuilderOpt {
+	return AddListenerConfigurer(
+		v3.ListenerMustConfigureFunc(func(l *envoy_listener.Listener) {
+			l.PerConnectionBufferLimitBytes = wrapperspb.UInt32(bytes)
+		}))
+}
+
+func EnableReusePort(enable bool) ListenerBuilderOpt {
+	return AddListenerConfigurer(
+		v3.ListenerMustConfigureFunc(func(l *envoy_listener.Listener) {
+			// TODO(jpeach) in Envoy 1.20, this field is deprecated in favor of EnableReusePort.
+			l.ReusePort = enable
+		}))
+}
+
+func EnableFreebind(enable bool) ListenerBuilderOpt {
+	return AddListenerConfigurer(
+		v3.ListenerMustConfigureFunc(func(l *envoy_listener.Listener) {
+			l.Freebind = wrapperspb.Bool(enable)
+		}))
+}
diff --git a/pkg/xds/envoy/listeners/v3/configurer.go b/pkg/xds/envoy/listeners/v3/configurer.go
index 09fb8fa4e..b6b7f9fc0 100644
--- a/pkg/xds/envoy/listeners/v3/configurer.go
+++ b/pkg/xds/envoy/listeners/v3/configurer.go
@@ -17,3 +17,27 @@ type FilterChainConfigurer interface {
 	// Configure configures a single aspect on a given Envoy filter chain.
 	Configure(filterChain *envoy_listener.FilterChain) error
 }
+
+// ListenerConfigureFunc adapts a configuration function to the
+// ListenerConfigurer interface.
+type ListenerConfigureFunc func(listener *envoy_listener.Listener) error
+
+func (f ListenerConfigureFunc) Configure(listener *envoy_listener.Listener) error {
+	if f != nil {
+		return f(listener)
+	}
+
+	return nil
+}
+
+// ListenerMustConfigureFunc adapts a configuration function that never
+// fails to the ListenerConfigurer interface.
+type ListenerMustConfigureFunc func(listener *envoy_listener.Listener)
+
+func (f ListenerMustConfigureFunc) Configure(listener *envoy_listener.Listener) error {
+	if f != nil {
+		f(listener)
+	}
+
+	return nil
+}
diff --git a/pkg/xds/sync/dataplane_watchdog.go b/pkg/xds/sync/dataplane_watchdog.go
index f84ba2951..449100276 100644
--- a/pkg/xds/sync/dataplane_watchdog.go
+++ b/pkg/xds/sync/dataplane_watchdog.go
@@ -70,7 +70,7 @@ func (d *DataplaneWatchdog) Sync() error {
 		d.proxyTypeSettled = true
 	}
 	switch d.dpType {
-	case mesh_proto.DataplaneProxyType, mesh_proto.GatewayProxyType:
+	case mesh_proto.DataplaneProxyType:
 		return d.syncDataplane()
 	case mesh_proto.IngressProxyType:
 		return d.syncIngress()
@@ -83,7 +83,7 @@ func (d *DataplaneWatchdog) Sync() error {
 func (d *DataplaneWatchdog) Cleanup() error {
 	proxyID := core_xds.FromResourceKey(d.key)
 	switch d.dpType {
-	case mesh_proto.DataplaneProxyType, mesh_proto.GatewayProxyType:
+	case mesh_proto.DataplaneProxyType:
 		return d.dataplaneReconciler.Clear(&proxyID)
 	case mesh_proto.IngressProxyType:
 		return d.ingressReconciler.Clear(&proxyID)


Test patch:
diff --git a/api/mesh/v1alpha1/dataplane_helpers_test.go b/api/mesh/v1alpha1/dataplane_helpers_test.go
index b15ca13ff..06374de1b 100644
--- a/api/mesh/v1alpha1/dataplane_helpers_test.go
+++ b/api/mesh/v1alpha1/dataplane_helpers_test.go
@@ -356,6 +356,42 @@ var _ = Describe("Dataplane with inbound", func() {
 	})
 })
 
+var _ = Describe("Dataplane classification", func() {
+	Describe("with normal networking", func() {
+		It("should be a dataplane", func() {
+			dp := Dataplane{
+				Networking: &Dataplane_Networking{},
+			}
+			Expect(dp.IsGateway()).To(BeFalse())
+			Expect(dp.IsIngress()).To(BeFalse())
+		})
+	})
+
+	Describe("with gateway networking", func() {
+		It("should be a gateway", func() {
+			gw := Dataplane{
+				Networking: &Dataplane_Networking{
+					Gateway: &Dataplane_Networking_Gateway{},
+				},
+			}
+			Expect(gw.IsGateway()).To(BeTrue())
+			Expect(gw.IsIngress()).To(BeFalse())
+		})
+	})
+
+	Describe("with ingress networking", func() {
+		It("should be an ingress gateway", func() {
+			in := Dataplane{
+				Networking: &Dataplane_Networking{
+					Ingress: &Dataplane_Networking_Ingress{},
+				},
+			}
+			Expect(in.IsGateway()).To(BeFalse())
+			Expect(in.IsIngress()).To(BeTrue())
+		})
+	})
+})
+
 var _ = Describe("Dataplane with gateway", func() {
 	d := Dataplane{
 		Networking: &Dataplane_Networking{
diff --git a/app/kuma-dp/cmd/run_test.go b/app/kuma-dp/cmd/run_test.go
index 1bd9dcffe..18f3715d8 100644
--- a/app/kuma-dp/cmd/run_test.go
+++ b/app/kuma-dp/cmd/run_test.go
@@ -359,4 +359,25 @@ var _ = Describe("run", func() {
 		Expect(err).To(HaveOccurred())
 		Expect(err.Error()).To(ContainSubstring("--name and --mesh cannot be specified when dataplane definition is provided"))
 	})
+
+	It("should fail when the proxy type is unknown", func() {
+		// given
+		cmd := NewRootCmd(DefaultRootContext())
+		cmd.SetArgs([]string{
+			"run",
+			"--cp-address", "http://localhost:1234",
+			"--binary-path", filepath.Join("testdata", "envoy-mock.sleep.sh"),
+			"--dataplane-file", filepath.Join("testdata", "dataplane_template.yaml"),
+			"--dns-coredns-path", filepath.Join("testdata", "coredns-mock.sleep.sh"),
+			"--proxy-type", "phoney",
+		})
+
+		// when
+		err := cmd.Execute()
+
+		// then
+		Expect(err).To(HaveOccurred())
+		Expect(err.Error()).To(ContainSubstring("invalid proxy type"))
+	})
+
 })
diff --git a/pkg/config/app/kuma-dp/config_test.go b/pkg/config/app/kuma-dp/config_test.go
index 506e3f76c..d5c1d6eb5 100644
--- a/pkg/config/app/kuma-dp/config_test.go
+++ b/pkg/config/app/kuma-dp/config_test.go
@@ -137,4 +137,15 @@ var _ = Describe("Config", func() {
 		fmt.Println(err.Error())
 		Expect(err.Error()).To(Equal(`Invalid configuration: .ControlPlane is not valid: .Retry is not valid: .Backoff must be a positive duration; .Dataplane is not valid: .ProxyType is not valid: not-a-proxy is not a valid proxy type; .Mesh must be non-empty; .Name must be non-empty; .DrainTime must be positive; .DataplaneRuntime is not valid: .BinaryPath must be non-empty`))
 	})
+
+	It("should ensure the proxy type is supported", func() {
+		// given
+		cfg := kuma_dp.Config{}
+
+		Expect(config.Load(filepath.Join("testdata", "valid-config.input.yaml"), &cfg)).Should(Succeed())
+
+		// when
+		cfg.Dataplane.ProxyType = string("gateway")
+		Expect(cfg.Validate()).ShouldNot(Succeed())
+	})
 })
diff --git a/pkg/xds/envoy/listeners/v3/http_route_configurer_test.go b/pkg/xds/envoy/listeners/v3/http_route_configurer_test.go
index 99f63882e..99f53bcf6 100644
--- a/pkg/xds/envoy/listeners/v3/http_route_configurer_test.go
+++ b/pkg/xds/envoy/listeners/v3/http_route_configurer_test.go
@@ -56,9 +56,7 @@ var _ = Describe("HttpScopedRouteConfigurer", func() {
 			InboundListener("inbound", "127.0.0.1", 99, xds.SocketAddressProtocolTCP),
 			FilterChain(NewFilterChainBuilder(envoy_common.APIV3).Configure(
 				HttpConnectionManager("inbound", false),
-				FilterChainBuilderOptFunc(func(c *FilterChainBuilderConfig) {
-					c.AddV3(&HttpScopedRouteConfigurer{})
-				}),
+				AddFilterChainConfigurer(&HttpScopedRouteConfigurer{}),
 			)),
 		).Build()
 
diff --git a/pkg/xds/envoy/listeners/v3/listener_configurer_test.go b/pkg/xds/envoy/listeners/v3/listener_configurer_test.go
new file mode 100644
index 000000000..08c25b6ee
--- /dev/null
+++ b/pkg/xds/envoy/listeners/v3/listener_configurer_test.go
@@ -0,0 +1,68 @@
+package v3_test
+
+import (
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/ginkgo/extensions/table"
+	. "github.com/onsi/gomega"
+
+	util_proto "github.com/kumahq/kuma/pkg/util/proto"
+	"github.com/kumahq/kuma/pkg/xds/envoy"
+	. "github.com/kumahq/kuma/pkg/xds/envoy/listeners"
+	v3 "github.com/kumahq/kuma/pkg/xds/envoy/listeners/v3"
+)
+
+var _ = Describe("Miscellaneous Listener configurers", func() {
+
+	type testCase struct {
+		opt      ListenerBuilderOpt
+		expected string
+	}
+
+	DescribeTable("should generate proper Envoy config",
+		func(given testCase) {
+			// when
+			listener := NewListenerBuilder(envoy.APIV3)
+
+			listener.Configure(given.opt)
+
+			// then
+			resource, err := listener.Build()
+			Expect(err).ToNot(HaveOccurred())
+
+			// when
+			actual, err := util_proto.ToYAML(resource)
+			Expect(err).ToNot(HaveOccurred())
+			// and
+			Expect(actual).To(MatchYAML(given.expected))
+		},
+		Entry("noop 1", testCase{
+			opt:      AddListenerConfigurer(v3.ListenerConfigureFunc(nil)),
+			expected: "{}",
+		}),
+		Entry("noop 2", testCase{
+			opt:      AddListenerConfigurer(v3.ListenerMustConfigureFunc(nil)),
+			expected: "{}",
+		}),
+		Entry("connection buffer limit", testCase{
+			opt:      ConnectionBufferLimit(123),
+			expected: "perConnectionBufferLimitBytes: 123",
+		}),
+		Entry("reuse port enabled", testCase{
+			opt:      EnableReusePort(true),
+			expected: "reusePort: true",
+		}),
+		Entry("reuse port disabled", testCase{
+			opt:      EnableReusePort(false),
+			expected: "{}",
+		}),
+		Entry("enable freebind", testCase{
+			opt:      EnableFreebind(true),
+			expected: "freebind: true",
+		}),
+		Entry("disable freebind", testCase{
+			opt:      EnableFreebind(false),
+			expected: "freebind: false",
+		}),
+	)
+
+})