fix(xds): backwards compatibility on access logs paths (backport of #…

…7662) (#7694) Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com> Co-authored-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>
kumahq · Sep 7, 2023 · 0daca1a · 0daca1a
1 parent 3f9f729
commit 0daca1a
Show file tree

Hide file tree

Showing 5 changed files with 64 additions and 31 deletions.
diff --git a/pkg/core/xds/metadata.go b/pkg/core/xds/metadata.go
@@ -142,7 +142,7 @@ func (m *DataplaneMetadata) GetVersion() *mesh_proto.Version {
 	return m.Version
 }
 
-func DataplaneMetadataFromXdsMetadata(xdsMetadata *structpb.Struct, tmpDir string) *DataplaneMetadata {
+func DataplaneMetadataFromXdsMetadata(xdsMetadata *structpb.Struct, tmpDir string, dpKey model.ResourceKey) *DataplaneMetadata {
 	// Be extra careful here about nil checks since xdsMetadata is a "user" input.
 	// Even if we know that something should not be nil since we are generating metadata,
 	// the DiscoveryRequest can still be crafted manually to crash the CP.
@@ -178,11 +178,9 @@ func DataplaneMetadataFromXdsMetadata(xdsMetadata *structpb.Struct, tmpDir strin
 	if xdsMetadata.Fields[FieldAccessLogSocketPath] != nil {
 		metadata.AccessLogSocketPath = xdsMetadata.Fields[FieldAccessLogSocketPath].GetStringValue()
 		metadata.MetricsSocketPath = xdsMetadata.Fields[FieldMetricsSocketPath].GetStringValue()
-	} else if metadata.Resource != nil {
-		name := metadata.Resource.GetMeta().GetName()
-		mesh := metadata.Resource.GetMeta().GetMesh()
-		metadata.AccessLogSocketPath = AccessLogSocketName(tmpDir, name, mesh)
-		metadata.MetricsSocketPath = MetricsHijackerSocketName(tmpDir, name, mesh)
+	} else {
+		metadata.AccessLogSocketPath = AccessLogSocketName(tmpDir, dpKey.Name, dpKey.Mesh)
+		metadata.MetricsSocketPath = MetricsHijackerSocketName(tmpDir, dpKey.Name, dpKey.Mesh)
 	}
 
 	if xdsMetadata.Fields[FieldMetricsCertPath] != nil {

diff --git a/pkg/core/xds/metadata_test.go b/pkg/core/xds/metadata_test.go
@@ -9,6 +9,7 @@ import (
 
 	mesh_proto "github.com/kumahq/kuma/api/mesh/v1alpha1"
 	core_mesh "github.com/kumahq/kuma/pkg/core/resources/apis/mesh"
+	core_model "github.com/kumahq/kuma/pkg/core/resources/model"
 	"github.com/kumahq/kuma/pkg/core/resources/model/rest"
 	"github.com/kumahq/kuma/pkg/core/xds"
 	"github.com/kumahq/kuma/pkg/test/matchers"
@@ -25,14 +26,20 @@ var _ = Describe("DataplaneMetadataFromXdsMetadata", func() {
 	DescribeTable("should parse metadata",
 		func(given testCase) {
 			// when
-			metadata := xds.DataplaneMetadataFromXdsMetadata(given.node, "/tmp")
+			metadata := xds.DataplaneMetadataFromXdsMetadata(given.node, "/tmp", core_model.ResourceKey{
+				Name: "dp-1",
+				Mesh: "mesh",
+			})
 
 			// then
 			Expect(*metadata).To(Equal(given.expected))
 		},
 		Entry("from empty node", testCase{
-			node:     &structpb.Struct{},
-			expected: xds.DataplaneMetadata{},
+			node: &structpb.Struct{},
+			expected: xds.DataplaneMetadata{
+				AccessLogSocketPath: "/tmp/kuma-al-dp-1-mesh.sock",
+				MetricsSocketPath:   "/tmp/kuma-mh-dp-1-mesh.sock",
+			},
 		}),
 		Entry("from non-empty node", testCase{
 			node: &structpb.Struct{
@@ -91,7 +98,9 @@ var _ = Describe("DataplaneMetadataFromXdsMetadata", func() {
 				},
 			},
 			expected: xds.DataplaneMetadata{
-				DynamicMetadata: map[string]string{},
+				AccessLogSocketPath: "/tmp/kuma-al-dp-1-mesh.sock",
+				MetricsSocketPath:   "/tmp/kuma-mh-dp-1-mesh.sock",
+				DynamicMetadata:     map[string]string{},
 			},
 		}),
 	)
@@ -121,7 +130,27 @@ var _ = Describe("DataplaneMetadataFromXdsMetadata", func() {
 		}
 
 		// when
-		metadata := xds.DataplaneMetadataFromXdsMetadata(node, "/tmp")
+		metadata := xds.DataplaneMetadataFromXdsMetadata(node, "/tmp", core_model.ResourceKey{
+			Name: "dp-1",
+			Mesh: "mesh",
+		})
+
+		// then
+		Expect(metadata.AccessLogSocketPath).To(Equal("/tmp/kuma-al-dp-1-mesh.sock"))
+		Expect(metadata.MetricsSocketPath).To(Equal("/tmp/kuma-mh-dp-1-mesh.sock"))
+	})
+
+	It("should fallback to service side generated paths without dpp in metadata", func() { // remove with https://github.com/kumahq/kuma/issues/7220
+		// given
+		node := &structpb.Struct{
+			Fields: map[string]*structpb.Value{},
+		}
+
+		// when
+		metadata := xds.DataplaneMetadataFromXdsMetadata(node, "/tmp", core_model.ResourceKey{
+			Name: "dp-1",
+			Mesh: "mesh",
+		})
 
 		// then
 		Expect(metadata.AccessLogSocketPath).To(Equal("/tmp/kuma-al-dp-1-mesh.sock"))
@@ -154,7 +183,10 @@ var _ = Describe("DataplaneMetadataFromXdsMetadata", func() {
 		}
 
 		// when
-		metadata := xds.DataplaneMetadataFromXdsMetadata(node, "/tmp")
+		metadata := xds.DataplaneMetadataFromXdsMetadata(node, "/tmp", core_model.ResourceKey{
+			Name: "dp-1",
+			Mesh: "mesh",
+		})
 
 		// then
 		// We don't want to validate KumaDpVersion.KumaCpCompatible

diff --git a/pkg/xds/auth/callbacks.go b/pkg/xds/auth/callbacks.go
@@ -118,7 +118,11 @@ func (a *authCallbacks) stream(streamID core_xds.StreamID, req util_xds.Discover
 	}
 
 	if s.resource == nil {
-		md := core_xds.DataplaneMetadataFromXdsMetadata(req.Metadata(), os.TempDir())
+		proxyId, err := core_xds.ParseProxyIdFromString(req.NodeId())
+		if err != nil {
+			return stream{}, errors.Wrap(err, "invalid node ID")
+		}
+		md := core_xds.DataplaneMetadataFromXdsMetadata(req.Metadata(), os.TempDir(), proxyId.ToResourceKey())
 		res, err := a.resource(user.Ctx(s.ctx, user.ControlPlane), md, req.NodeId())
 		if err != nil {
 			return stream{}, err

diff --git a/pkg/xds/server/callbacks/dataplane_callbacks.go b/pkg/xds/server/callbacks/dataplane_callbacks.go
@@ -96,7 +96,7 @@ func (d *xdsCallbacks) OnStreamRequest(streamID core_xds.StreamID, request util_
 		return errors.Wrap(err, "invalid node ID")
 	}
 	dpKey := proxyId.ToResourceKey()
-	metadata := core_xds.DataplaneMetadataFromXdsMetadata(request.Metadata(), os.TempDir())
+	metadata := core_xds.DataplaneMetadataFromXdsMetadata(request.Metadata(), os.TempDir(), dpKey)
 	if metadata == nil {
 		return errors.New("metadata in xDS Node cannot be nil")
 	}

diff --git a/pkg/xds/server/callbacks/dataplane_status_tracker.go b/pkg/xds/server/callbacks/dataplane_status_tracker.go
@@ -129,26 +129,25 @@ func (c *dataplaneStatusTracker) OnStreamRequest(streamID int64, req util_xds.Di
 	defer state.mu.Unlock()
 
 	if state.dataplaneId == (core_model.ResourceKey{}) {
-		var dpType core_model.ResourceType
-		md := core_xds.DataplaneMetadataFromXdsMetadata(req.Metadata(), os.TempDir())
-
-		// If the dataplane was started with a resource YAML, then it
-		// will be serialized in the node metadata and we would know
-		// the underlying type directly. Since that is optional, we
-		// can't depend on it here, so we map from the proxy type,
-		// which is guaranteed.
-		switch md.GetProxyType() {
-		case mesh_proto.IngressProxyType:
-			dpType = core_mesh.ZoneIngressType
-		case mesh_proto.DataplaneProxyType:
-			dpType = core_mesh.DataplaneType
-		case mesh_proto.EgressProxyType:
-			dpType = core_mesh.ZoneEgressType
-		}
-
 		// Infer the Dataplane ID.
 		if proxyId, err := core_xds.ParseProxyIdFromString(req.NodeId()); err == nil {
 			state.dataplaneId = proxyId.ToResourceKey()
+			var dpType core_model.ResourceType
+			md := core_xds.DataplaneMetadataFromXdsMetadata(req.Metadata(), os.TempDir(), state.dataplaneId)
+
+			// If the dataplane was started with a resource YAML, then it
+			// will be serialized in the node metadata and we would know
+			// the underlying type directly. Since that is optional, we
+			// can't depend on it here, so we map from the proxy type,
+			// which is guaranteed.
+			switch md.GetProxyType() {
+			case mesh_proto.IngressProxyType:
+				dpType = core_mesh.ZoneIngressType
+			case mesh_proto.DataplaneProxyType:
+				dpType = core_mesh.DataplaneType
+			case mesh_proto.EgressProxyType:
+				dpType = core_mesh.ZoneEgressType
+			}
 
 			log := statusTrackerLog.WithValues(
 				"proxyName", state.dataplaneId.Name,