Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading