From a59e06f1575115737949b6751c18b0279adc9ef6 Mon Sep 17 00:00:00 2001 From: Jakub Dyszkiewicz Date: Thu, 7 Sep 2023 08:41:02 +0200 Subject: [PATCH] fix(xds): backwards compatibility on access logs paths (#7662) Signed-off-by: Jakub Dyszkiewicz --- pkg/core/xds/metadata.go | 15 ++++ pkg/core/xds/metadata_test.go | 86 ++++++++++++++++++- pkg/xds/auth/callbacks.go | 8 ++ .../server/callbacks/dataplane_callbacks.go | 4 + .../callbacks/dataplane_status_tracker.go | 19 ++++ 5 files changed, 129 insertions(+), 3 deletions(-) diff --git a/pkg/core/xds/metadata.go b/pkg/core/xds/metadata.go index 2d10ad5ffd52..f9d7cae08dd3 100644 --- a/pkg/core/xds/metadata.go +++ b/pkg/core/xds/metadata.go @@ -133,7 +133,11 @@ func (m *DataplaneMetadata) GetVersion() *mesh_proto.Version { return m.Version } +<<<<<<< HEAD func DataplaneMetadataFromXdsMetadata(xdsMetadata *structpb.Struct) *DataplaneMetadata { +======= +func DataplaneMetadataFromXdsMetadata(xdsMetadata *structpb.Struct, tmpDir string, dpKey model.ResourceKey) *DataplaneMetadata { +>>>>>>> 27ea0f00c (fix(xds): backwards compatibility on access logs paths (#7662)) // 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. @@ -164,6 +168,17 @@ func DataplaneMetadataFromXdsMetadata(xdsMetadata *structpb.Struct) *DataplaneMe "value", value) } } +<<<<<<< HEAD +======= + // TODO Backward compat for 2 versions after 2.4 prior to 2.4 these were not passed in the metadata https://github.com/kumahq/kuma/issues/7220 (remove the parameter tmpDir of the function too) + if xdsMetadata.Fields[FieldAccessLogSocketPath] != nil { + metadata.AccessLogSocketPath = xdsMetadata.Fields[FieldAccessLogSocketPath].GetStringValue() + metadata.MetricsSocketPath = xdsMetadata.Fields[FieldMetricsSocketPath].GetStringValue() + } else { + metadata.AccessLogSocketPath = AccessLogSocketName(tmpDir, dpKey.Name, dpKey.Mesh) + metadata.MetricsSocketPath = MetricsHijackerSocketName(tmpDir, dpKey.Name, dpKey.Mesh) + } +>>>>>>> 27ea0f00c (fix(xds): backwards compatibility on access logs paths (#7662)) if listValue := xdsMetadata.Fields[fieldFeatures]; listValue != nil { metadata.Features = Features{} diff --git a/pkg/core/xds/metadata_test.go b/pkg/core/xds/metadata_test.go index 176a66aa9d76..ed03dd77dee7 100644 --- a/pkg/core/xds/metadata_test.go +++ b/pkg/core/xds/metadata_test.go @@ -6,6 +6,12 @@ import ( "google.golang.org/protobuf/types/known/structpb" mesh_proto "github.com/kumahq/kuma/api/mesh/v1alpha1" +<<<<<<< HEAD +======= + 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" +>>>>>>> 27ea0f00c (fix(xds): backwards compatibility on access logs paths (#7662)) "github.com/kumahq/kuma/pkg/core/xds" "github.com/kumahq/kuma/pkg/test/matchers" util_proto "github.com/kumahq/kuma/pkg/util/proto" @@ -20,14 +26,24 @@ var _ = Describe("DataplaneMetadataFromXdsMetadata", func() { DescribeTable("should parse metadata", func(given testCase) { // when +<<<<<<< HEAD metadata := xds.DataplaneMetadataFromXdsMetadata(given.node) +======= + metadata := xds.DataplaneMetadataFromXdsMetadata(given.node, "/tmp", core_model.ResourceKey{ + Name: "dp-1", + Mesh: "mesh", + }) +>>>>>>> 27ea0f00c (fix(xds): backwards compatibility on access logs paths (#7662)) // 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{ @@ -74,11 +90,68 @@ 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{}, }, }), ) +<<<<<<< HEAD +======= + It("should fallback to service side generated paths", func() { // remove with https://github.com/kumahq/kuma/issues/7220 + // given + dpJSON, err := json.Marshal(rest.From.Resource(&core_mesh.DataplaneResource{ + Meta: &test_model.ResourceMeta{Mesh: "mesh", Name: "dp-1"}, + Spec: &mesh_proto.Dataplane{ + Networking: &mesh_proto.Dataplane_Networking{ + Address: "123.40.2.2", + Inbound: []*mesh_proto.Dataplane_Networking_Inbound{ + {Address: "10.0.0.1", Port: 8080, Tags: map[string]string{"kuma.io/service": "foo"}}, + }, + }, + }, + })) + Expect(err).ToNot(HaveOccurred()) + node := &structpb.Struct{ + Fields: map[string]*structpb.Value{ + "dataplane.resource": { + Kind: &structpb.Value_StringValue{ + StringValue: string(dpJSON), + }, + }, + }, + } + + // 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")) + 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")) + Expect(metadata.MetricsSocketPath).To(Equal("/tmp/kuma-mh-dp-1-mesh.sock")) + }) + +>>>>>>> 27ea0f00c (fix(xds): backwards compatibility on access logs paths (#7662)) It("should parse version", func() { // this has to be separate test because Equal does not work on proto // given version := &mesh_proto.Version{ @@ -105,7 +178,14 @@ var _ = Describe("DataplaneMetadataFromXdsMetadata", func() { } // when +<<<<<<< HEAD metadata := xds.DataplaneMetadataFromXdsMetadata(node) +======= + metadata := xds.DataplaneMetadataFromXdsMetadata(node, "/tmp", core_model.ResourceKey{ + Name: "dp-1", + Mesh: "mesh", + }) +>>>>>>> 27ea0f00c (fix(xds): backwards compatibility on access logs paths (#7662)) // then // We don't want to validate KumaDpVersion.KumaCpCompatible diff --git a/pkg/xds/auth/callbacks.go b/pkg/xds/auth/callbacks.go index 4578111a4e23..ce6db7b45d02 100644 --- a/pkg/xds/auth/callbacks.go +++ b/pkg/xds/auth/callbacks.go @@ -117,7 +117,15 @@ func (a *authCallbacks) stream(streamID core_xds.StreamID, req util_xds.Discover } if s.resource == nil { +<<<<<<< HEAD md := core_xds.DataplaneMetadataFromXdsMetadata(req.Metadata()) +======= + 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()) +>>>>>>> 27ea0f00c (fix(xds): backwards compatibility on access logs paths (#7662)) 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 index d8c2d004c371..4223082b9e50 100644 --- a/pkg/xds/server/callbacks/dataplane_callbacks.go +++ b/pkg/xds/server/callbacks/dataplane_callbacks.go @@ -95,7 +95,11 @@ func (d *xdsCallbacks) OnStreamRequest(streamID core_xds.StreamID, request util_ return errors.Wrap(err, "invalid node ID") } dpKey := proxyId.ToResourceKey() +<<<<<<< HEAD metadata := core_xds.DataplaneMetadataFromXdsMetadata(request.Metadata()) +======= + metadata := core_xds.DataplaneMetadataFromXdsMetadata(request.Metadata(), os.TempDir(), dpKey) +>>>>>>> 27ea0f00c (fix(xds): backwards compatibility on access logs paths (#7662)) 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 index d0a262fe41df..0694cd3c891b 100644 --- a/pkg/xds/server/callbacks/dataplane_status_tracker.go +++ b/pkg/xds/server/callbacks/dataplane_status_tracker.go @@ -128,6 +128,7 @@ func (c *dataplaneStatusTracker) OnStreamRequest(streamID int64, req util_xds.Di defer state.mu.Unlock() if state.dataplaneId == (core_model.ResourceKey{}) { +<<<<<<< HEAD var dpType core_model.ResourceType md := core_xds.DataplaneMetadataFromXdsMetadata(req.Metadata()) @@ -145,9 +146,27 @@ func (c *dataplaneStatusTracker) OnStreamRequest(streamID int64, req util_xds.Di dpType = core_mesh.ZoneEgressType } +======= +>>>>>>> 27ea0f00c (fix(xds): backwards compatibility on access logs paths (#7662)) // 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,