Repository: kumahq/kuma
Pull request #2424 (2021-07-25 21:32:15)

chore(*) separate proxy template resolvers (backport #2394)
This is an automatic backport of pull request #2394 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/pkg/xds/generator/proxy_template.go b/pkg/xds/generator/proxy_template.go
index 885324d33..195795ada 100644
--- a/pkg/xds/generator/proxy_template.go
+++ b/pkg/xds/generator/proxy_template.go
@@ -5,17 +5,17 @@ import (
 
 	"github.com/pkg/errors"
 
-	"github.com/kumahq/kuma/pkg/xds/generator/modifications"
-
-	kuma_mesh "github.com/kumahq/kuma/api/mesh/v1alpha1"
+	mesh_proto "github.com/kumahq/kuma/api/mesh/v1alpha1"
 	mesh_core "github.com/kumahq/kuma/pkg/core/resources/apis/mesh"
 	model "github.com/kumahq/kuma/pkg/core/xds"
 	util_envoy "github.com/kumahq/kuma/pkg/util/envoy"
 	xds_context "github.com/kumahq/kuma/pkg/xds/context"
+	"github.com/kumahq/kuma/pkg/xds/generator/modifications"
+	"github.com/kumahq/kuma/pkg/xds/template"
 )
 
 type ProxyTemplateGenerator struct {
-	ProxyTemplate *kuma_mesh.ProxyTemplate
+	ProxyTemplate *mesh_proto.ProxyTemplate
 }
 
 func (g *ProxyTemplateGenerator) Generate(ctx xds_context.Context, proxy *model.Proxy) (*model.ResourceSet, error) {
@@ -44,7 +44,7 @@ func (g *ProxyTemplateGenerator) Generate(ctx xds_context.Context, proxy *model.
 const OriginProxyTemplateRaw = "proxy-template-raw"
 
 type ProxyTemplateRawSource struct {
-	Resources []*kuma_mesh.ProxyTemplateRawResource
+	Resources []*mesh_proto.ProxyTemplateRawResource
 }
 
 func (s *ProxyTemplateRawSource) Generate(_ xds_context.Context, proxy *model.Proxy) (*model.ResourceSet, error) {
@@ -64,7 +64,17 @@ func (s *ProxyTemplateRawSource) Generate(_ xds_context.Context, proxy *model.Pr
 	return resources, nil
 }
 
-var predefinedProfiles = make(map[string]ResourceGenerator)
+type ProxyTemplateProfileSource struct {
+	ProfileName string
+}
+
+func (s *ProxyTemplateProfileSource) Generate(ctx xds_context.Context, proxy *model.Proxy) (*model.ResourceSet, error) {
+	g, ok := predefinedProfiles[s.ProfileName]
+	if !ok {
+		return nil, fmt.Errorf("profile{name=%q}: unknown profile", s.ProfileName)
+	}
+	return g.Generate(ctx, proxy)
+}
 
 func NewDefaultProxyProfile() ResourceGenerator {
 	return CompositeResourceGenerator{
@@ -80,6 +90,19 @@ func NewDefaultProxyProfile() ResourceGenerator {
 	}
 }
 
+// DefaultTemplateResolver is the default template resolver that xDS
+// generators fall back to if they are otherwise unable to determine which
+// ProxyTemplate resource to apply. Plugins may modify this variable.
+var DefaultTemplateResolver template.ProxyTemplateResolver = &template.StaticProxyTemplateResolver{
+	Template: &mesh_proto.ProxyTemplate{
+		Conf: &mesh_proto.ProxyTemplate_Conf{
+			Imports: []string{mesh_core.ProfileDefaultProxy},
+		},
+	},
+}
+
+var predefinedProfiles = make(map[string]ResourceGenerator)
+
 func init() {
 	RegisterProfile(mesh_core.ProfileDefaultProxy, NewDefaultProxyProfile())
 	RegisterProfile(IngressProxy, CompositeResourceGenerator{AdminProxyGenerator{}, IngressGenerator{}})
@@ -88,15 +111,3 @@ func init() {
 func RegisterProfile(profileName string, generator ResourceGenerator) {
 	predefinedProfiles[profileName] = generator
 }
-
-type ProxyTemplateProfileSource struct {
-	ProfileName string
-}
-
-func (s *ProxyTemplateProfileSource) Generate(ctx xds_context.Context, proxy *model.Proxy) (*model.ResourceSet, error) {
-	g, ok := predefinedProfiles[s.ProfileName]
-	if !ok {
-		return nil, fmt.Errorf("profile{name=%q}: unknown profile", s.ProfileName)
-	}
-	return g.Generate(ctx, proxy)
-}
diff --git a/pkg/xds/server/v3/components.go b/pkg/xds/server/v3/components.go
index e3d09b637..e00a98ddd 100644
--- a/pkg/xds/server/v3/components.go
+++ b/pkg/xds/server/v3/components.go
@@ -7,6 +7,7 @@ import (
 	envoy_service_discovery "github.com/envoyproxy/go-control-plane/envoy/service/discovery/v3"
 	envoy_server "github.com/envoyproxy/go-control-plane/pkg/server/v3"
 
+	mesh_proto "github.com/kumahq/kuma/api/mesh/v1alpha1"
 	"github.com/kumahq/kuma/pkg/core"
 	core_model "github.com/kumahq/kuma/pkg/core/resources/model"
 	core_runtime "github.com/kumahq/kuma/pkg/core/runtime"
@@ -18,6 +19,7 @@ import (
 	"github.com/kumahq/kuma/pkg/xds/cache/mesh"
 	xds_context "github.com/kumahq/kuma/pkg/xds/context"
 	envoy_common "github.com/kumahq/kuma/pkg/xds/envoy"
+	"github.com/kumahq/kuma/pkg/xds/generator"
 	xds_metrics "github.com/kumahq/kuma/pkg/xds/metrics"
 	xds_callbacks "github.com/kumahq/kuma/pkg/xds/server/callbacks"
 	xds_sync "github.com/kumahq/kuma/pkg/xds/sync"
@@ -71,25 +73,37 @@ func RegisterXDS(
 }
 
 func DefaultReconciler(rt core_runtime.Runtime, xdsContext v3.XdsContext) xds_sync.SnapshotReconciler {
+	resolver := xds_template.SequentialResolver(
+		&xds_template.SimpleProxyTemplateResolver{
+			ReadOnlyResourceManager: rt.ReadOnlyResourceManager(),
+		},
+		generator.DefaultTemplateResolver,
+	)
+
 	return &reconciler{
 		&templateSnapshotGenerator{
-			ResourceSetHooks: rt.XDSHooks().ResourceSetHooks(),
-			ProxyTemplateResolver: &xds_template.SimpleProxyTemplateResolver{
-				ReadOnlyResourceManager: rt.ReadOnlyResourceManager(),
-				DefaultProxyTemplate:    xds_template.DefaultProxyTemplate,
-			},
+			ResourceSetHooks:      rt.XDSHooks().ResourceSetHooks(),
+			ProxyTemplateResolver: resolver,
 		},
 		&simpleSnapshotCacher{xdsContext.Hasher(), xdsContext.Cache()},
 	}
 }
 
 func DefaultIngressReconciler(rt core_runtime.Runtime, xdsContext v3.XdsContext) xds_sync.SnapshotReconciler {
+	resolver := &xds_template.StaticProxyTemplateResolver{
+		Template: &mesh_proto.ProxyTemplate{
+			Conf: &mesh_proto.ProxyTemplate_Conf{
+				Imports: []string{
+					generator.IngressProxy,
+				},
+			},
+		},
+	}
+
 	return &reconciler{
 		generator: &templateSnapshotGenerator{
-			ResourceSetHooks: rt.XDSHooks().ResourceSetHooks(),
-			ProxyTemplateResolver: &xds_template.StaticProxyTemplateResolver{
-				Template: xds_template.IngressProxyTemplate,
-			},
+			ResourceSetHooks:      rt.XDSHooks().ResourceSetHooks(),
+			ProxyTemplateResolver: resolver,
 		},
 		cacher: &simpleSnapshotCacher{xdsContext.Hasher(), xdsContext.Cache()},
 	}
diff --git a/pkg/xds/template/proxy_template.go b/pkg/xds/template/proxy_template.go
deleted file mode 100644
index 4aa4c5461..000000000
--- a/pkg/xds/template/proxy_template.go
+++ /dev/null
@@ -1,25 +0,0 @@
-package template
-
-import (
-	kuma_mesh "github.com/kumahq/kuma/api/mesh/v1alpha1"
-	core_mesh "github.com/kumahq/kuma/pkg/core/resources/apis/mesh"
-	"github.com/kumahq/kuma/pkg/xds/generator"
-)
-
-var (
-	DefaultProxyTemplate = &kuma_mesh.ProxyTemplate{
-		Conf: &kuma_mesh.ProxyTemplate_Conf{
-			Imports: []string{
-				core_mesh.ProfileDefaultProxy,
-			},
-		},
-	}
-
-	IngressProxyTemplate = &kuma_mesh.ProxyTemplate{
-		Conf: &kuma_mesh.ProxyTemplate_Conf{
-			Imports: []string{
-				generator.IngressProxy,
-			},
-		},
-	}
-)
diff --git a/pkg/xds/template/proxy_template_resolver.go b/pkg/xds/template/proxy_template_resolver.go
index 2c4d7a25d..cf6f89fdb 100644
--- a/pkg/xds/template/proxy_template_resolver.go
+++ b/pkg/xds/template/proxy_template_resolver.go
@@ -22,8 +22,7 @@ type ProxyTemplateResolver interface {
 }
 
 type SimpleProxyTemplateResolver struct {
-	manager.ReadOnlyResourceManager
-	DefaultProxyTemplate *mesh_proto.ProxyTemplate
+	ReadOnlyResourceManager manager.ReadOnlyResourceManager
 }
 
 func (r *SimpleProxyTemplateResolver) GetTemplate(proxy *model.Proxy) *mesh_proto.ProxyTemplate {
@@ -44,8 +43,9 @@ func (r *SimpleProxyTemplateResolver) GetTemplate(proxy *model.Proxy) *mesh_prot
 		log.V(2).Info("found the best matching ProxyTemplate", "proxytemplate", core_model.MetaToResourceKey(bestMatchTemplate.GetMeta()))
 		return bestMatchTemplate.(*mesh_core.ProxyTemplateResource).Spec
 	}
-	log.V(2).Info("falling back to the default ProxyTemplate since there is no best match", "templates", templateList.Items)
-	return r.DefaultProxyTemplate
+
+	log.V(2).Info("no matching ProxyTemplate")
+	return nil
 }
 
 type StaticProxyTemplateResolver struct {
@@ -55,3 +55,22 @@ type StaticProxyTemplateResolver struct {
 func (r *StaticProxyTemplateResolver) GetTemplate(proxy *model.Proxy) *mesh_proto.ProxyTemplate {
 	return r.Template
 }
+
+type sequentialResolver []ProxyTemplateResolver
+
+func (s sequentialResolver) GetTemplate(proxy *model.Proxy) *mesh_proto.ProxyTemplate {
+	for _, r := range s {
+		if t := r.GetTemplate(proxy); t != nil {
+			return t
+		}
+	}
+
+	return nil
+}
+
+// SequentialResolver returns a new ProxyTemplate resolver that applies
+// each of the resolvers given as arguments in turn. The result of the
+// first successful resolver is returned.
+func SequentialResolver(r ...ProxyTemplateResolver) ProxyTemplateResolver {
+	return sequentialResolver(r)
+}


Test patch:
diff --git a/pkg/xds/server/v3/snapshot_generator_test.go b/pkg/xds/server/v3/snapshot_generator_test.go
index 504decdcc..7142cbac0 100644
--- a/pkg/xds/server/v3/snapshot_generator_test.go
+++ b/pkg/xds/server/v3/snapshot_generator_test.go
@@ -7,18 +7,18 @@ import (
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 
-	"github.com/kumahq/kuma/pkg/test/matchers"
-
 	mesh_proto "github.com/kumahq/kuma/api/mesh/v1alpha1"
 	mesh_core "github.com/kumahq/kuma/pkg/core/resources/apis/mesh"
 	"github.com/kumahq/kuma/pkg/core/resources/manager"
 	model "github.com/kumahq/kuma/pkg/core/xds"
 	"github.com/kumahq/kuma/pkg/plugins/resources/memory"
+	"github.com/kumahq/kuma/pkg/test/matchers"
 	test_model "github.com/kumahq/kuma/pkg/test/resources/model"
 	util_cache_v3 "github.com/kumahq/kuma/pkg/util/cache/v3"
 	util_proto "github.com/kumahq/kuma/pkg/util/proto"
 	xds_context "github.com/kumahq/kuma/pkg/xds/context"
 	envoy_common "github.com/kumahq/kuma/pkg/xds/envoy"
+	"github.com/kumahq/kuma/pkg/xds/generator"
 	"github.com/kumahq/kuma/pkg/xds/template"
 )
 
@@ -26,10 +26,12 @@ var _ = Describe("Reconcile", func() {
 	Describe("templateSnapshotGenerator", func() {
 
 		gen := templateSnapshotGenerator{
-			ProxyTemplateResolver: &template.SimpleProxyTemplateResolver{
-				ReadOnlyResourceManager: manager.NewResourceManager(memory.NewStore()),
-				DefaultProxyTemplate:    template.DefaultProxyTemplate,
-			},
+			ProxyTemplateResolver: template.SequentialResolver(
+				&template.SimpleProxyTemplateResolver{
+					ReadOnlyResourceManager: manager.NewResourceManager(memory.NewStore()),
+				},
+				generator.DefaultTemplateResolver,
+			),
 		}
 
 		It("Generate Snapshot per Envoy Node", func() {
diff --git a/pkg/xds/template/proxy_template_resolver_test.go b/pkg/xds/template/proxy_template_resolver_test.go
index 3012a4ca9..2412553e6 100644
--- a/pkg/xds/template/proxy_template_resolver_test.go
+++ b/pkg/xds/template/proxy_template_resolver_test.go
@@ -18,7 +18,7 @@ import (
 
 var _ = Describe("Reconcile", func() {
 	Describe("SimpleProxyTemplateResolver", func() {
-		It("should fallback to the default ProxyTemplate when there are no other candidates", func() {
+		It("should return nil when there no other candidates", func() {
 			// given
 			proxy := &model.Proxy{
 				Dataplane: &mesh_core.DataplaneResource{
@@ -31,14 +31,13 @@ var _ = Describe("Reconcile", func() {
 			// setup
 			resolver := &SimpleProxyTemplateResolver{
 				ReadOnlyResourceManager: manager.NewResourceManager(memory.NewStore()),
-				DefaultProxyTemplate:    &mesh_proto.ProxyTemplate{},
 			}
 
 			// when
 			actual := resolver.GetTemplate(proxy)
 
 			// then
-			Expect(actual).To(BeIdenticalTo(resolver.DefaultProxyTemplate))
+			Expect(actual).To(BeNil())
 		})
 
 		It("should use Client to list ProxyTemplates in the same Mesh as Dataplane", func() {
@@ -95,7 +94,6 @@ var _ = Describe("Reconcile", func() {
 
 			resolver := &SimpleProxyTemplateResolver{
 				ReadOnlyResourceManager: manager.NewResourceManager(memStore),
-				DefaultProxyTemplate:    &mesh_proto.ProxyTemplate{},
 			}
 
 			// when
@@ -109,7 +107,7 @@ var _ = Describe("Reconcile", func() {
 			}))
 		})
 
-		It("should fallback to the default ProxyTemplate if there are no custom templates in a given Mesh", func() {
+		It("should return nil if there are no custom templates in a given Mesh", func() {
 			// given
 			proxy := &model.Proxy{
 				Dataplane: &mesh_core.DataplaneResource{
@@ -122,14 +120,13 @@ var _ = Describe("Reconcile", func() {
 			// setup
 			resolver := &SimpleProxyTemplateResolver{
 				ReadOnlyResourceManager: manager.NewResourceManager(memory.NewStore()),
-				DefaultProxyTemplate:    &mesh_proto.ProxyTemplate{},
 			}
 
 			// when
 			actual := resolver.GetTemplate(proxy)
 
 			// then
-			Expect(actual).To(BeIdenticalTo(resolver.DefaultProxyTemplate))
+			Expect(actual).To(BeNil())
 		})
 
 	})