fix: Filter mechanism chain element works incorrectly in remote case (#…

…576) * Revert "fix: Filter mechanism chain element works incorrectly in remote case (#575)" This reverts commit 39537ac. Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com> * Fix 573 Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com> * fix typo Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>
networkservicemesh · Nov 9, 2020 · 74848aa · 74848aa
1 parent c677c7a
commit 74848aa
Show file tree

Hide file tree

Showing 10 changed files with 99 additions and 320 deletions.
diff --git a/pkg/networkservice/common/filtermechanisms/README.md b/pkg/networkservice/common/filtermechanisms/README.md
diff --git a/pkg/networkservice/common/filtermechanisms/const.go b/pkg/networkservice/common/filtermechanisms/const.go
diff --git a/pkg/networkservice/common/filtermechanisms/options.go b/pkg/networkservice/common/filtermechanisms/options.go
diff --git a/pkg/networkservice/common/filtermechanisms/server.go b/pkg/networkservice/common/filtermechanisms/server.go
@@ -21,6 +21,8 @@ package filtermechanisms
 import (
 	"context"
 
+	"github.com/networkservicemesh/sdk/pkg/registry/common/interpose"
+
 	"github.com/networkservicemesh/sdk/pkg/tools/clienturlctx"
 
 	"github.com/golang/protobuf/ptypes/empty"
@@ -33,30 +35,22 @@ import (
 )
 
 type filterMechanismsServer struct {
-	urls           endpointurls.Set
-	localThreshold int
+	nses endpointurls.Map
 }
 
 // NewServer - filters out remote mechanisms if connection is received from a unix file socket, otherwise filters
-// out local mechanisms.
-func NewServer(registryServer *registry.NetworkServiceEndpointRegistryServer, options ...Option) networkservice.NetworkServiceServer {
-	result := &filterMechanismsServer{
-		localThreshold: defaultThreshold,
-	}
-	for _, applyOption := range options {
-		applyOption(result)
-	}
-	*registryServer = endpointurls.NewNetworkServiceEndpointRegistryServer(&result.urls)
+// out local mechanisms
+func NewServer(registryServer *registry.NetworkServiceEndpointRegistryServer) networkservice.NetworkServiceServer {
+	result := &filterMechanismsServer{}
+	*registryServer = endpointurls.NewNetworkServiceEndpointRegistryServer(&result.nses)
 	return result
 }
 
 func (f *filterMechanismsServer) Request(ctx context.Context, request *networkservice.NetworkServiceRequest) (*networkservice.Connection, error) {
 	u := clienturlctx.ClientURL(ctx)
-
-	if _, ok := f.urls.Load(*u); ok {
-		filteredMechanisms := filterMechanismsByCls(request.GetMechanismPreferences(), cls.LOCAL)
-		if len(filteredMechanisms) > 0 {
-			request.MechanismPreferences = filteredMechanisms
+	if name, ok := f.nses.Load(*u); ok {
+		if !interpose.Is(name) {
+			request.MechanismPreferences = filterMechanismsByCls(request.GetMechanismPreferences(), cls.LOCAL)
 		}
 	} else {
 		request.MechanismPreferences = filterMechanismsByCls(request.GetMechanismPreferences(), cls.REMOTE)

diff --git a/pkg/networkservice/common/filtermechanisms/server_test.go b/pkg/networkservice/common/filtermechanisms/server_test.go
@@ -21,8 +21,6 @@ import (
 	"net/url"
 	"testing"
 
-	"github.com/networkservicemesh/sdk/pkg/networkservice/core/next"
-
 	"github.com/networkservicemesh/api/pkg/api/networkservice"
 	"github.com/networkservicemesh/api/pkg/api/networkservice/mechanisms/cls"
 	"github.com/networkservicemesh/api/pkg/api/networkservice/mechanisms/kernel"
@@ -36,72 +34,6 @@ import (
 	"github.com/networkservicemesh/sdk/pkg/tools/clienturlctx"
 )
 
-func TestFilterMechanisms_RemoteExternal(t *testing.T) {
-	request := &networkservice.NetworkServiceRequest{
-		MechanismPreferences: []*networkservice.Mechanism{
-			{
-				Cls:  cls.REMOTE,
-				Type: vxlan.MECHANISM,
-			},
-			{
-				Cls:  cls.LOCAL,
-				Type: kernel.MECHANISM,
-			},
-		},
-		Connection: &networkservice.Connection{
-			Path: &networkservice.Path{
-				PathSegments: make([]*networkservice.PathSegment, 3),
-			},
-		},
-	}
-
-	var localRegistryServer registry.NetworkServiceEndpointRegistryServer
-	localServer := filtermechanisms.NewServer(&localRegistryServer)
-	var remoteRegistryServer registry.NetworkServiceEndpointRegistryServer
-	remoteServer := filtermechanisms.NewServer(&remoteRegistryServer, filtermechanisms.WithExternalThreshold())
-	u := &url.URL{Path: "test"}
-	_, _ = remoteRegistryServer.Register(context.Background(), &registry.NetworkServiceEndpoint{
-		Url: u.String(),
-	})
-	_, err := next.NewNetworkServiceServer(localServer, remoteServer).Request(clienturlctx.WithClientURL(context.Background(), u), request)
-	require.NoError(t, err)
-	require.Len(t, request.MechanismPreferences, 1)
-	require.Equal(t, cls.REMOTE, request.MechanismPreferences[0].Cls)
-}
-
-func TestFilterMechanisms_Remote(t *testing.T) {
-	request := &networkservice.NetworkServiceRequest{
-		MechanismPreferences: []*networkservice.Mechanism{
-			{
-				Cls:  cls.REMOTE,
-				Type: vxlan.MECHANISM,
-			},
-			{
-				Cls:  cls.LOCAL,
-				Type: kernel.MECHANISM,
-			},
-		},
-		Connection: &networkservice.Connection{
-			Path: &networkservice.Path{
-				PathSegments: make([]*networkservice.PathSegment, 5),
-			},
-		},
-	}
-
-	var localRegistryServer registry.NetworkServiceEndpointRegistryServer
-	localServer := filtermechanisms.NewServer(&localRegistryServer)
-	var remoteRegistryServer registry.NetworkServiceEndpointRegistryServer
-	remoteServer := filtermechanisms.NewServer(&remoteRegistryServer)
-	u := &url.URL{Path: "test"}
-	_, _ = remoteRegistryServer.Register(context.Background(), &registry.NetworkServiceEndpoint{
-		Url: u.String(),
-	})
-	_, err := next.NewNetworkServiceServer(localServer, remoteServer).Request(clienturlctx.WithClientURL(context.Background(), u), request)
-	require.NoError(t, err)
-	require.Len(t, request.MechanismPreferences, 1)
-	require.Equal(t, cls.REMOTE, request.MechanismPreferences[0].Cls)
-}
-
 func TestFilterMechanismsServer_Request(t *testing.T) {
 	request := func() *networkservice.NetworkServiceRequest {
 		return &networkservice.NetworkServiceRequest{
@@ -127,6 +59,7 @@ func TestFilterMechanismsServer_Request(t *testing.T) {
 	}
 	samples := []struct {
 		Name         string
+		EndpointName string
 		ClientURL    *url.URL
 		RegisterURLs []url.URL
 		ClsResult    string
@@ -140,12 +73,25 @@ func TestFilterMechanismsServer_Request(t *testing.T) {
 					Host:   "localhost:5000",
 				},
 			},
-			ClsResult: cls.LOCAL,
+			EndpointName: "nse-1",
+			ClsResult:    cls.LOCAL,
 		},
 		{
-			Name:      "Remote mechanisms",
-			ClientURL: &url.URL{Scheme: "tcp", Host: "localhost:5000"},
-			ClsResult: cls.REMOTE,
+			Name:         "Remote mechanisms",
+			ClientURL:    &url.URL{Scheme: "tcp", Host: "localhost:5000"},
+			EndpointName: "nse-1",
+			ClsResult:    cls.REMOTE,
+		},
+		{
+			Name:         "Pass mechanisms to forwarder",
+			ClientURL:    &url.URL{Scheme: "tcp", Host: "localhost:5000"},
+			EndpointName: "interpose-nse#nse-1",
+			RegisterURLs: []url.URL{
+				{
+					Scheme: "tcp",
+					Host:   "localhost:5000",
+				},
+			},
 		},
 	}
 
@@ -154,7 +100,8 @@ func TestFilterMechanismsServer_Request(t *testing.T) {
 		s := filtermechanisms.NewServer(&registryServer)
 		for _, u := range sample.RegisterURLs {
 			_, err := registryServer.Register(context.Background(), &registry.NetworkServiceEndpoint{
-				Url: u.String(),
+				Name: sample.EndpointName,
+				Url:  u.String(),
 			})
 			require.NoError(t, err)
 		}
@@ -163,8 +110,13 @@ func TestFilterMechanismsServer_Request(t *testing.T) {
 		_, err := s.Request(ctx, req)
 		require.NoError(t, err)
 		require.NotEmpty(t, req.MechanismPreferences)
-		for _, m := range req.MechanismPreferences {
-			require.Equal(t, sample.ClsResult, m.Cls, "filtermechanisms chain element should properly filter mechanisms")
+
+		if sample.ClsResult != "" {
+			for _, m := range req.MechanismPreferences {
+				require.Equal(t, sample.ClsResult, m.Cls, "filtermechanisms chain element should properly filter mechanisms")
+			}
+		} else {
+			require.Equal(t, request().String(), req.String())
 		}
 	}
 }
diff --git a/pkg/registry/common/endpointurls/gen.go b/pkg/registry/common/endpointurls/gen.go
@@ -20,8 +20,8 @@ import (
 	"sync"
 )
 
-//go:generate go-syncmap -output sync_set.gen.go -type Set<net/url.URL,struct{}>
+//go:generate go-syncmap -output sync_map.gen.go -type Map<net/url.URL,string>
 
-// Set is like a Go map[url.URL]struct{} but is safe for concurrent use
+// Map is like a Go map[url.URL]string but is safe for concurrent use
 // by multiple goroutines without additional locking or coordination
-type Set sync.Map
+type Map sync.Map
diff --git a/pkg/registry/common/endpointurls/server.go b/pkg/registry/common/endpointurls/server.go
@@ -14,7 +14,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// Package endpointurls provides registry.NetworkServiceEndpointRegistryServer that can be injected in the chain of registry.NetworkServiceEndpointRegistryServer to get an actual set of registry.NetworkServiceEndpoint URLs.
+// Package endpointurls provides registry.NetworkServiceEndpointRegistryServer that can be injected in the chain of registry.NetworkServiceEndpointRegistryServer to get an actual nses of registry.NetworkServiceEndpoint URLs.
 package endpointurls
 
 import (
@@ -28,12 +28,12 @@ import (
 )
 
 type endpointURLsServer struct {
-	set *Set
+	nses *Map
 }
 
 func (e *endpointURLsServer) Register(ctx context.Context, endpoint *registry.NetworkServiceEndpoint) (*registry.NetworkServiceEndpoint, error) {
 	if u, err := url.Parse(endpoint.Url); err == nil {
-		e.set.Store(*u, struct{}{})
+		e.nses.Store(*u, endpoint.Name)
 	}
 	return next.NetworkServiceEndpointRegistryServer(ctx).Register(ctx, endpoint)
 }
@@ -44,12 +44,12 @@ func (e *endpointURLsServer) Find(query *registry.NetworkServiceEndpointQuery, s
 
 func (e *endpointURLsServer) Unregister(ctx context.Context, endpoint *registry.NetworkServiceEndpoint) (*empty.Empty, error) {
 	if u, err := url.Parse(endpoint.Url); err == nil {
-		e.set.Delete(*u)
+		e.nses.Delete(*u)
 	}
 	return next.NetworkServiceEndpointRegistryServer(ctx).Unregister(ctx, endpoint)
 }
 
-// NewNetworkServiceEndpointRegistryServer returns new registry.NetworkServiceEndpointRegistryServer with injected endpoint urls set
-func NewNetworkServiceEndpointRegistryServer(set *Set) registry.NetworkServiceEndpointRegistryServer {
-	return &endpointURLsServer{set: set}
+// NewNetworkServiceEndpointRegistryServer returns new registry.NetworkServiceEndpointRegistryServer with injected endpoint urls nses
+func NewNetworkServiceEndpointRegistryServer(m *Map) registry.NetworkServiceEndpointRegistryServer {
+	return &endpointURLsServer{nses: m}
 }