Fix setid

pkg/networkservice/chains/nsmgr/server.go

@@ -94,10 +94,7 @@ func NewServer(ctx context.Context, nsmRegistration *registryapi.NetworkServiceE
 
 	nseRegistry := newRemoteNSEServer(registryCC)
 	if nseRegistry == nil {
-		nseRegistry = chain_registry.NewNetworkServiceEndpointRegistryServer(
-			setid.NewNetworkServiceEndpointRegistryServer(),  // If no remote registry then assign ID.
-			memory.NewNetworkServiceEndpointRegistryServer(), // Memory registry to store result inside.
-		)
+		nseRegistry = memory.NewNetworkServiceEndpointRegistryServer() // Memory registry to store result inside.
 	}
 
 	nseClient := next.NewNetworkServiceEndpointRegistryClient(
@@ -136,6 +133,7 @@ func NewServer(ctx context.Context, nsmRegistration *registryapi.NetworkServiceE
 	nseChain := chain_registry.NewNamedNetworkServiceEndpointRegistryServer(
 		nsmRegistration.Name+".NetworkServiceEndpointRegistry",
 		expire.NewNetworkServiceEndpointRegistryServer(time.Minute),
+		setid.NewNetworkServiceEndpointRegistryServer(),           // Assign ID
 		registry_recvfd.NewNetworkServiceEndpointRegistryServer(), // Allow to receive a passed files
 		urlsRegistryServer,
 		interposeRegistry,         // Store cross connect NSEs

pkg/networkservice/chains/nsmgr/server_test.go

@@ -42,6 +42,7 @@ import (
 	"github.com/networkservicemesh/sdk/pkg/networkservice/common/mechanisms/kernel"
 	"github.com/networkservicemesh/sdk/pkg/networkservice/core/chain"
 	"github.com/networkservicemesh/sdk/pkg/networkservice/core/next"
+	"github.com/networkservicemesh/sdk/pkg/networkservice/utils/inject/injecterror"
 	"github.com/networkservicemesh/sdk/pkg/tools/opentracing"
 	"github.com/networkservicemesh/sdk/pkg/tools/sandbox"
 )
@@ -412,9 +413,7 @@ func TestNSMGR_PassThroughRemote(t *testing.T) {
 					clienturl.NewServer(domain.Nodes[i].NSMgr.URL),
 					connect.NewServer(ctx,
 						sandbox.NewCrossConnectClientFactory(sandbox.GenerateTestToken,
-							newPassTroughClient(
-								fmt.Sprintf("my-service-remote-%v", k-1),
-								fmt.Sprintf("endpoint-%v", k-1)),
+							newPassTroughClient(fmt.Sprintf("my-service-remote-%v", k-1)),
 							kernel.NewClient()),
 						append(opentracing.WithTracingDial(), grpc.WithBlock(), grpc.WithInsecure())...,
 					),
@@ -474,9 +473,7 @@ func TestNSMGR_PassThroughLocal(t *testing.T) {
 					clienturl.NewServer(domain.Nodes[0].NSMgr.URL),
 					connect.NewServer(ctx,
 						sandbox.NewCrossConnectClientFactory(sandbox.GenerateTestToken,
-							newPassTroughClient(
-								fmt.Sprintf("my-service-remote-%v", k-1),
-								fmt.Sprintf("endpoint-%v", k-1)),
+							newPassTroughClient(fmt.Sprintf("my-service-remote-%v", k-1)),
 							kernel.NewClient()),
 						append(opentracing.WithTracingDial(), grpc.WithBlock(), grpc.WithInsecure())...,
 					),
@@ -513,6 +510,119 @@ func TestNSMGR_PassThroughLocal(t *testing.T) {
 	require.Equal(t, 5*(nsesCount-1)+5, len(conn.Path.PathSegments))
 }
 
+func TestNSMGR_ShouldCorrectlyAddForwardersWithSameNames(t *testing.T) {
+	defer goleak.VerifyNone(t, goleak.IgnoreCurrent())
+
+	ctx, cancel := context.WithTimeout(context.Background(), time.Second*5)
+	defer cancel()
+
+	domain := sandbox.NewBuilder(t).
+		SetNodesCount(1).
+		SetRegistryProxySupplier(nil).
+		SetNodeSetup(nil).
+		SetContext(ctx).
+		Build()
+	defer domain.Cleanup()
+
+	forwarderReg := &registry.NetworkServiceEndpoint{
+		Name: "forwarder",
+	}
+
+	nseReg := &registry.NetworkServiceEndpoint{
+		Name:                "endpoint",
+		NetworkServiceNames: []string{"service"},
+	}
+
+	// 1. Add forwarders
+	forwarder1Reg := forwarderReg.Clone()
+	_, err := domain.Nodes[0].NewForwarder(ctx, forwarder1Reg, sandbox.GenerateTestToken)
+	require.NoError(t, err)
+
+	forwarder2Reg := forwarderReg.Clone()
+	_, err = domain.Nodes[0].NewForwarder(ctx, forwarder2Reg, sandbox.GenerateTestToken)
+	require.NoError(t, err)
+
+	forwarder3Reg := forwarderReg.Clone()
+	_, err = domain.Nodes[0].NewForwarder(ctx, forwarder3Reg, sandbox.GenerateTestToken)
+	require.NoError(t, err)
+
+	// 2. Wait for refresh
+	<-time.After(sandbox.DefaultRegistryExpiryDuration)
+
+	testNSEAndClient(ctx, t, domain, nseReg.Clone())
+
+	// 3. Delete first forwarder
+	_, err = domain.Nodes[0].ForwarderRegistryClient.Unregister(ctx, forwarder1Reg)
+	require.NoError(t, err)
+
+	testNSEAndClient(ctx, t, domain, nseReg.Clone())
+
+	// 4. Delete last forwarder
+	_, err = domain.Nodes[0].ForwarderRegistryClient.Unregister(ctx, forwarder3Reg)
+	require.NoError(t, err)
+
+	testNSEAndClient(ctx, t, domain, nseReg.Clone())
+
+	_, err = domain.Nodes[0].ForwarderRegistryClient.Unregister(ctx, forwarder2Reg)
+	require.NoError(t, err)
+}
+
+func TestNSMGR_ShouldCorrectlyAddEndpointsWithSameNames(t *testing.T) {
+	defer goleak.VerifyNone(t, goleak.IgnoreCurrent())
+
+	ctx, cancel := context.WithTimeout(context.Background(), time.Second*5)
+	defer cancel()
+
+	domain := sandbox.NewBuilder(t).
+		SetNodesCount(1).
+		SetRegistryProxySupplier(nil).
+		SetContext(ctx).
+		Build()
+	defer domain.Cleanup()
+
+	nseReg := &registry.NetworkServiceEndpoint{
+		Name: "endpoint",
+	}
+
+	nsc := domain.Nodes[0].NewClient(ctx, sandbox.GenerateTestToken)
+
+	// 1. Add endpoints
+	nse1Reg := nseReg.Clone()
+	nse1Reg.NetworkServiceNames = []string{"service-1"}
+	_, err := domain.Nodes[0].NewEndpoint(ctx, nse1Reg, sandbox.GenerateTestToken)
+	require.NoError(t, err)
+
+	nse2Reg := nseReg.Clone()
+	nse2Reg.NetworkServiceNames = []string{"service-2"}
+	_, err = domain.Nodes[0].NewEndpoint(ctx, nse2Reg, sandbox.GenerateTestToken)
+	require.NoError(t, err)
+
+	// 2. Wait for refresh
+	<-time.After(sandbox.DefaultRegistryExpiryDuration)
+
+	// 3. Request
+	_, err = nsc.Request(ctx, &networkservice.NetworkServiceRequest{
+		Connection: &networkservice.Connection{
+			NetworkService: "service-1",
+		},
+	})
+	require.NoError(t, err)
+
+	_, err = nsc.Request(ctx, &networkservice.NetworkServiceRequest{
+		Connection: &networkservice.Connection{
+			NetworkService: "service-2",
+		},
+	})
+	require.NoError(t, err)
+
+	// 3. Delete endpoints
+	_, err = domain.Nodes[0].ForwarderRegistryClient.Unregister(ctx, nse1Reg)
+	require.NoError(t, err)
+
+	_, err = domain.Nodes[0].ForwarderRegistryClient.Unregister(ctx, nse2Reg)
+	require.NoError(t, err)
+}
+
 func TestNSMGR_ShouldCleanAllClientAndEndpointGoroutines(t *testing.T) {
 	defer goleak.VerifyNone(t, goleak.IgnoreCurrent())
 
@@ -588,26 +698,24 @@ func testNSEAndClient(
 }
 
 type passThroughClient struct {
-	networkService             string
-	networkServiceEndpointName string
+	networkService string
 }
 
-func newPassTroughClient(networkService, networkServiceEndpointName string) *passThroughClient {
+func newPassTroughClient(networkService string) *passThroughClient {
 	return &passThroughClient{
-		networkService:             networkService,
-		networkServiceEndpointName: networkServiceEndpointName,
+		networkService: networkService,
 	}
 }
 
 func (c *passThroughClient) Request(ctx context.Context, request *networkservice.NetworkServiceRequest, opts ...grpc.CallOption) (*networkservice.Connection, error) {
 	request.Connection.NetworkService = c.networkService
-	request.Connection.NetworkServiceEndpointName = c.networkServiceEndpointName
+	request.Connection.NetworkServiceEndpointName = ""
 	return next.Client(ctx).Request(ctx, request, opts...)
 }
 
 func (c *passThroughClient) Close(ctx context.Context, conn *networkservice.Connection, opts ...grpc.CallOption) (*empty.Empty, error) {
 	conn.NetworkService = c.networkService
-	conn.NetworkServiceEndpointName = c.networkServiceEndpointName
+	conn.NetworkServiceEndpointName = ""
 	return next.Client(ctx).Close(ctx, conn, opts...)
 }
 
@@ -643,15 +751,6 @@ func (c *restartingEndpoint) Close(ctx context.Context, connection *networkservi
 	return next.Client(ctx).Close(ctx, connection)
 }
 
-type busyEndpoint struct{}
-
-func (c *busyEndpoint) Request(ctx context.Context, request *networkservice.NetworkServiceRequest) (*networkservice.Connection, error) {
-	return nil, errors.New("sorry, endpoint is busy, try again later")
-}
-
-func (c *busyEndpoint) Close(ctx context.Context, connection *networkservice.Connection) (*empty.Empty, error) {
-	return nil, errors.New("sorry, endpoint is busy, try again later")
-}
 func newBusyEndpoint() networkservice.NetworkServiceServer {
-	return new(busyEndpoint)
+	return injecterror.NewServer(errors.New("sorry, endpoint is busy, try again later"))
 }

pkg/networkservice/common/discover/server_test.go

@@ -22,23 +22,21 @@ import (
 	"testing"
 	"time"
 
-	"github.com/networkservicemesh/api/pkg/api/networkservice/payload"
-
-	"github.com/networkservicemesh/sdk/pkg/tools/clienturlctx"
-
-	"github.com/networkservicemesh/sdk/pkg/registry/common/memory"
-	"github.com/networkservicemesh/sdk/pkg/registry/common/setid"
-	"github.com/networkservicemesh/sdk/pkg/registry/core/adapters"
+	"github.com/stretchr/testify/require"
+	"go.uber.org/goleak"
 
 	"github.com/networkservicemesh/api/pkg/api/networkservice"
+	"github.com/networkservicemesh/api/pkg/api/networkservice/payload"
 	"github.com/networkservicemesh/api/pkg/api/registry"
-	"github.com/stretchr/testify/require"
-	"go.uber.org/goleak"
 
 	"github.com/networkservicemesh/sdk/pkg/networkservice/common/discover"
 	"github.com/networkservicemesh/sdk/pkg/networkservice/core/next"
 	"github.com/networkservicemesh/sdk/pkg/networkservice/utils/checks/checkcontext"
+	"github.com/networkservicemesh/sdk/pkg/registry/common/memory"
+	"github.com/networkservicemesh/sdk/pkg/registry/common/setid"
+	"github.com/networkservicemesh/sdk/pkg/registry/core/adapters"
 	registrynext "github.com/networkservicemesh/sdk/pkg/registry/core/next"
+	"github.com/networkservicemesh/sdk/pkg/tools/clienturlctx"
 )
 
 func endpoints() []*registry.NetworkServiceEndpoint {
@@ -444,10 +442,7 @@ func TestMatchExactService(t *testing.T) {
 func TestMatchExactEndpoint(t *testing.T) {
 	defer goleak.VerifyNone(t, goleak.IgnoreCurrent())
 
-	nseServer := registrynext.NewNetworkServiceEndpointRegistryServer(
-		setid.NewNetworkServiceEndpointRegistryServer(),
-		memory.NewNetworkServiceEndpointRegistryServer(),
-	)
+	nseServer := memory.NewNetworkServiceEndpointRegistryServer()
 
 	nseName := "final-endpoint"
 	u := "tcp://" + nseName

pkg/networkservice/common/filtermechanisms/server_test.go

@@ -1,4 +1,4 @@
-// Copyright (c) 2020 Doc.ai and/or its affiliates.
+// Copyright (c) 2020-2021 Doc.ai and/or its affiliates.
 //
 // SPDX-License-Identifier: Apache-2.0
 //
@@ -85,7 +85,7 @@ func TestFilterMechanismsServer_Request(t *testing.T) {
 		{
 			Name:         "Pass mechanisms to forwarder",
 			ClientURL:    &url.URL{Scheme: "tcp", Host: "localhost:5000"},
-			EndpointName: "interpose-nse#nse-1",
+			EndpointName: "nse-1#interpose-nse",
 			RegisterURLs: []url.URL{
 				{
 					Scheme: "tcp",

pkg/networkservice/common/interpose/server_test.go

@@ -1,4 +1,4 @@
-// Copyright (c) 2020 Doc.ai and/or its affiliates.
+// Copyright (c) 2020-2021 Doc.ai and/or its affiliates.
 //
 // SPDX-License-Identifier: Apache-2.0
 //
@@ -33,6 +33,9 @@ import (
 	"github.com/networkservicemesh/sdk/pkg/networkservice/core/adapters"
 	"github.com/networkservicemesh/sdk/pkg/networkservice/core/next"
 	"github.com/networkservicemesh/sdk/pkg/networkservice/utils/checks/checkcontext"
+	registryinterpose "github.com/networkservicemesh/sdk/pkg/registry/common/interpose"
+	registryadapters "github.com/networkservicemesh/sdk/pkg/registry/core/adapters"
+	registrynext "github.com/networkservicemesh/sdk/pkg/registry/core/next"
 	"github.com/networkservicemesh/sdk/pkg/tools/clienturlctx"
 )
 
@@ -43,8 +46,11 @@ func TestInterposeServer(t *testing.T) {
 	var interposeRegistry registry.NetworkServiceEndpointRegistryServer
 	interposeServer := interpose.NewServer(&interposeRegistry)
 
-	_, err := interposeRegistry.Register(context.TODO(), &registry.NetworkServiceEndpoint{
-		Name: "interpose-nse#",
+	_, err := registrynext.NewNetworkServiceEndpointRegistryClient(
+		registryinterpose.NewNetworkServiceEndpointRegistryClient(),
+		registryadapters.NetworkServiceEndpointServerToClient(interposeRegistry),
+	).Register(context.TODO(), &registry.NetworkServiceEndpoint{
+		Name: "forwarder",
 		Url:  crossNSEURL.String(),
 	})
 	require.NoError(t, err)
@@ -64,7 +70,7 @@ func TestInterposeServer(t *testing.T) {
 			}),
 		)),
 		adapters.NewServerToClient(next.NewNetworkServiceServer(
-			updatepath.NewServer("interpose-nse"),
+			updatepath.NewServer("forwarder"),
 		)),
 		adapters.NewServerToClient(next.NewNetworkServiceServer(
 			updatepath.NewServer("nsmgr"),

pkg/registry/common/interpose/name.go

@@ -18,14 +18,13 @@ package interpose
 
 import "strings"
 
-// namePrefix - a common prefix for all registered cross NSEs
-const namePrefix = "interpose-nse#"
+const nameSuffix = "#interpose-nse"
 
 func interposeName(name string) string {
-	return namePrefix + name
+	return name + nameSuffix
 }
 
 // Is returns true if passed name contains interpose identity
 func Is(name string) bool {
-	return strings.HasPrefix(name, namePrefix)
+	return strings.HasSuffix(name, nameSuffix)
 }

pkg/registry/common/interpose/server_test.go

@@ -31,7 +31,7 @@ import (
 )
 
 const (
-	namePrefix     = "interpose-nse#"
+	nameSuffix     = "#interpose-nse"
 	name           = "nse"
 	url            = "tcp://0.0.0.0"
 	commonResponse = "response"
@@ -45,11 +45,11 @@ var samples = []struct {
 	{
 		name: "interpose NSE",
 		in: &registry.NetworkServiceEndpoint{
-			Name: namePrefix + name,
+			Name: name + nameSuffix,
 			Url:  url,
 		},
 		out: &registry.NetworkServiceEndpoint{
-			Name: namePrefix + name,
+			Name: name + nameSuffix,
 			Url:  url,
 		},
 		isInMap: true,
@@ -66,7 +66,7 @@ var samples = []struct {
 	{
 		name: "invalid NSE",
 		in: &registry.NetworkServiceEndpoint{
-			Name: namePrefix + name,
+			Name: name + nameSuffix,
 		},
 		isInMap: false,
 		failure: true,