Skip to content

Commit

Permalink
fix(xds): backwards compatibility on access logs paths (#7662)
Browse files Browse the repository at this point in the history
Signed-off-by: Jakub Dyszkiewicz <[email protected]>
  • Loading branch information
jakubdyszkiewicz authored and kumahq[bot] committed Sep 7, 2023
1 parent 37d13eb commit a59e06f
Show file tree
Hide file tree
Showing 5 changed files with 129 additions and 3 deletions.
15 changes: 15 additions & 0 deletions pkg/core/xds/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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{}
Expand Down
86 changes: 83 additions & 3 deletions pkg/core/xds/metadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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{
Expand Down Expand Up @@ -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{
Expand All @@ -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
Expand Down
8 changes: 8 additions & 0 deletions pkg/xds/auth/callbacks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions pkg/xds/server/callbacks/dataplane_callbacks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
19 changes: 19 additions & 0 deletions pkg/xds/server/callbacks/dataplane_status_tracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())

Expand All @@ -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,
Expand Down

0 comments on commit a59e06f

Please sign in to comment.