networkservicemesh / sdk

Network Service Mesh sdk

This repo is for platform independent sdk code

For platform specific sdk code, please see the sdk-${platform} repos:

• sdk-vppagent
• sdk-kernel
• sdk-sriov

sdk's Issues

Create tests for /pkg/networkservice/common/timeout package

Very similar to /pkg/networkservice/common/refresh tests

Generalize authorize chain element

The current chain element looks only at the last token in the path chain. This is insufficient for the full spectrum of policy we are likely to want to enforce.

Expand on the authorize chain element by providing as its 'input' object:

1. The Connection
2. The TLSInfo (as retrieved by peer.FromContext for Server or grpc.Peer for the client)
3. Operation - One of Request/Close
4. Role - One of Client/Endpoint

Debug test failure for MacOS

https://github.com/networkservicemesh/sdk/pull/222/checks?check_run_id=609539466

Create exclude prefixes SDK chain elements.

We need exclude prefixes Chain element for server.

As well we need exclude prefixes application to listen for Kubernetes network prefixes and CRD and update resource. Chain element should periodically track resource state and re-read exclude prefixes.

Use goleak in each test to avoid goroutines leaks

Description

We've found one interesting problem with goleak uber-go/goleak#48, in short it can detect leaks from other tests.

Potential solution

We can avoid this problem if we'll use goleak tool in each test.

Update registry/next pkg with the latest patch for networkservice/next

Description

We've updated next logic for networkservice/next, networkservice/adapters with this 6a29a89, now we can apply this patch for https://github.com/networkservicemesh/sdk/tree/master/pkg/registry/core/next, https://github.com/networkservicemesh/sdk/tree/master/pkg/registry/core/adapters

Add go generate ./... check on CI

Description

We have go generating file https://github.com/networkservicemesh/sdk/blob/master/pkg/tools/callback/gen.go#L19 and we have not a check on CI for this. There is a possibility that we will not notice any problems with the relevance of the generated files

Solution

Add CI job go generate on CI.

Get rid of using go-heder directly on CI

Description

Now, go-header linter included in the set of golangci linters golangci/golangci-lint#1181. We need to wait for the new release and after that simplify our ci.yaml config. (This section will be removed https://github.com/networkservicemesh/sdk/blob/master/.github/workflows/ci.yaml#L174)

Adapters ignores context from adpated client/server

Problem statement

We need to store context from the adapter server/client and pass it to the next client/server to avoid the problem with losing
values from adapted server/client context.

Note: Here we are passing context https://github.com/networkservicemesh/sdk/blob/master/pkg/networkservice/core/adapters/client_to_server.go#L41
and actually all values from the chain are losing.

Solution

1. Use here reference to the context instead of boolean flag

   sdk/pkg/networkservice/core/adapters/context.go

   Line 32 in f833e64

   d := false

   and apply changes to fix compilation errors
2. Pass context from the adapted chain serve/client to the next client/server.
3. Add test to cover

Sometimes unit test 'TestNewClient_StopRefreshAtClose' has goroutine leak

Context

https://github.com/networkservicemesh/sdk/pull/227/checks?check_run_id=645773960

Failure logs

##[error]    leaks.go:78: found unexpected goroutines:
        [Goroutine 62 in state chan send, with github.com/networkservicemesh/sdk/pkg/networkservice/common/refresh_test.TestNewClient_StopRefreshAtClose.func1 on top of the stack:
        goroutine 62 [chan send]:
        github.com/networkservicemesh/sdk/pkg/networkservice/common/refresh_test.TestNewClient_StopRefreshAtClose.func1(0x18d8e20, 0xc000154120, 0xc000180280, 0x0, 0x0, 0x0, 0x1180c0b, 0xc000154140, 0xc000154120)
        	/Users/runner/runners/2.169.1/work/sdk/sdk/pkg/networkservice/common/refresh/client_test.go:97 +0x78
        github.com/networkservicemesh/sdk/pkg/networkservice/common/refresh_test.(*testRefresh).Request(0xc000010280, 0x18d8e20, 0xc000154120, 0xc000180280, 0x0, 0x0, 0x0, 0xc000154120, 0x18d21a0, 0xc000108b70)
        	/Users/runner/runners/2.169.1/work/sdk/sdk/pkg/networkservice/common/refresh/client_test.go:67 +0x96
        github.com/networkservicemesh/sdk/pkg/networkservice/core/next.(*nextClient).Request(0xc000108b70, 0x18d8f60, 0xc000152100, 0xc000180280, 0x0, 0x0, 0x0, 0x106989c, 0xc000152100, 0xc000048c00)
        	/Users/runner/runners/2.169.1/work/sdk/sdk/pkg/networkservice/core/next/client.go:70 +0x5fb
        github.com/networkservicemesh/sdk/pkg/networkservice/common/refresh.(*refreshClient).Request(0xc00000ee20, 0x18d8f60, 0xc000152100, 0xc000180280, 0x0, 0x0, 0x0, 0xc0001c8218, 0x1092790, 0xc0001c81c0)
        	/Users/runner/runners/2.169.1/work/sdk/sdk/pkg/networkservice/common/refresh/client.go:53 +0xaa
        github.com/networkservicemesh/sdk/pkg/networkservice/common/refresh.(*refreshClient).createTimer.func1()
        	/Users/runner/runners/2.169.1/work/sdk/sdk/pkg/networkservice/common/refresh/client.go:96 +0x183
        created by time.goFunc
        	/Users/runner/hostedtoolcache/go/1.13.4/x64/src/time/sleep.go:168 +0x52
        ]

Create DNSContext NetworkServiceClient chain element

Description

Previously we had added DNSContext server chain element: #74

We need also to create in pkg/networkservice/connectioncontext/dnscontext/ a NetworkServiceClient chain element appropriate for getting a DNSContext for clients.

This part was blocked by #48

Implement vl3 chain components

Motivation

We need to create vl3 chain components to add possible to build different vl3 NSEs based on SDK.

Related to networkservicemesh/networkservicemesh#2136

The main usage of these chain elements are:

• Registration of vl3 NSE in the floating registry
• Find and request all vl3 NSEs of the same type
  ...

Diagram of interaction

https://drive.google.com/file/d/1jq4AMOICUsL3zx-4-5SJ1P3UpxYO4V0U/view?usp=sharing // WIP

Interdomain: Add registry dnsresolve chain element to resolve passed domain to clienturl

Context

#235

Description

To add possible to connect proxy registry to remote registry we need to put into context URL to the remote target.

Solution

Add dnsresolve chain element that's simply will resolve passed domain to URL via DNS and pass it into context.

Registry: add resources cleanup chain element

Motivation

With the new registry model, we need to add a chain element to manage NSM resources(NSs,NSEs). This component should do next things:

1. Deleting NS if the registry has not NSE entries that provide this NS.
2. Deleting NSE if achieved expiration time.

In nsmgr chain, determine how to validate and filter connection context from nse response

nsmgr copies the connection context from the NSE response. Not all information should be copied. Determine what we should filter and implement a mechanism to do so.

Also, provide a way to validate the connection context. E.g. NSE ignoring exclude prefixes.

Update SDK to use the latest networkservicemesh/api dependency

Motivation

We've simplified registry API and now we need to update and fix build errors in SDK repository with the latest nsm/api dependency.

Empty Match does not evaluate properly unless last

The contract for Matches in the Network Service is that we evaluate each one from first to last until one matches. It appears this is not currently the case:

#116 (comment)

Unit test `TestNewClient_StopRefreshAtAnotherRequest` is not stable

Failure logs

=== FAIL: pkg/networkservice/common/refresh TestNewClient_StopRefreshAtAnotherRequest (0.19s)
    client_test.go:170: 
        	Error Trace:	client_test.go:170
        	Error:      	Condition never satisfied
        	Test:       	TestNewClient_StopRefreshAtAnotherRequest

Link to CI job

https://github.com/networkservicemesh/sdk/pull/257/checks?check_run_id=719395988

Create NetworkServiceRegistryClient to add podName/nodeName/clusterName labels to registrations

Create NetworkServiceRegistryClient to add podName/nodeName/clusterName labels to registrations.
Put it in sdk/pkg/registry/common/

Default Policy examples

We need default OPA Policies based upon the update in input found in #200

Among the examples we'd want:

1. Tokens valid - checks the validity of some or all of the tokens in the path.
2. No Tokens in path expired - none of the tokens in the chain are expired.
3. Last Token matches TLSInfo - checks to make sure that the Last Token (JWT) is signed by the same cert as found in TLSInfo.
4. Token.Aud for token n in path matches Token.Sub for n+1 token in chain

Add goleak for checking goroutines leaking on CI

https://github.com/uber-go/goleak - Goroutine leak detector to help avoid Goroutine leaks.

Create Cross connect chain elements

We need a cross connect chain element to perform call from NSmgr to cross connect and handle requests from cross connect and and forward connection to endpoint or next nsmgr.

Add interdomain chain elements

Motivation

We need to rethink and migrate our interdomain staff from monorepo to SDK.

Spec doc link

https://docs.google.com/document/d/1JOPV-9C7cUBAdDyPtHE5BgLHRqc_OJq4YOPy7_51gyo/edit?usp=sharing

TODO

Split tasks on separate issues

Add OPA policies for NSM registries

Motivation

Probably we need to add authorization policies for NSM registries to cover scenarios such as

1. NSE/NSMgr/xconnNSE should be able to unregister only itself. NSE/NSMgr/xconn NSE should not be able to unregister another NSE/NSMgr/xconn NSE.
2. Only NSMgr/vl3 NSE should be able to search NSE in the registry.
3. Only NSMgr and other registries can use Bulk API.

Problem of implementation

We need to somehow pass Path data or other authorization data to registries.

Potential solution

We can use pkg grpc/metadata for passing authorization data for registries.

import "google.golang.org/grpc/metadata"
...  
ctx = metadata.NewContext( 
    ctx, 
    metadata.Pairs("path-data", "data"), 
) 

Add tools/xconnectnse to help register/find/unregister xconnectnses

Motivation

We need to add possible to register xconnect nses in the registry.

Solution

Add tools/xconnectnse pkg and add a function that expects URL to registry and URL to xconnect nse.

package xconnectnse

Register(client, registry.NetworkServiceRegistryClient, name string,  xconnectnseURL *url.URL, ) error {
//TODO implement function here
}

Unregister(client, registry.NetworkServiceRegistryClient, name string,  xconnectnseURL *url.URL, ) error {
//TODO implement function here
}

Find(client registry.NetworkServiceRegistryClient) []registry.NetworkServiceEdnpoint {
}

OR

Add registry/xconnnse client chain element that will simply fill needed fields and pass it to next client (real registry client)

Define and implement example/test OPA use cases.

Initial OPA implementation is currently in the repo with a simple use case.

1. Define example use cases that we want to cover with OPA
2. For each use case, attempt to implement the policy. Document the results and gaps.
3. For each use case where a gap, expand OPA support.

Flaky test TestNewClient_MissingConnectionsInInit in pkg/networkservice/common/heal

Sometimes PASS, sometimes FAIL. @alex-yust reproduced it locally too
attach test log:

=== Failed
=== FAIL: pkg/networkservice/common/heal TestNewClient_MissingConnectionsInInit (0.03s)
time="2020-02-21T08:21:46Z" level=info msg="Creating new eventReceiver"
time="2020-02-21T08:21:46Z" level=info msg="==--> *healClient.Request() span:{}"
time="2020-02-21T08:21:46Z" level=info msg=">><<-- request={\"connection\":{\"id\":\"conn-1\",\"network_service\":\"ns-1\"}} span={}"
time="2020-02-21T08:21:46Z" level=info msg=">><<-- response={\"id\":\"conn-1\",\"network_service\":\"ns-1\"} span={}"
time="2020-02-21T08:21:46Z" level=info msg="==--> *healClient.Request() span:{}"
time="2020-02-21T08:21:46Z" level=info msg=">><<-- request={\"connection\":{\"id\":\"conn-2\",\"network_service\":\"ns-2\"}} span={}"
time="2020-02-21T08:21:46Z" level=info msg=">><<-- response={\"id\":\"conn-2\",\"network_service\":\"ns-2\"} span={}"
    client_test.go:238: 
        	Error Trace:	client_test.go:238
        	Error:      	Should be true
        	Test:       	TestNewClient_MissingConnectionsInInit

OPA should support multiple tokens in the chain

OPA currently only reasons on a single token in the chain. It will be useful to have OPA support multiple tokens so that it can reason about elements deeper in the chain.

Context: #40 (comment)

Add registry memory chain components

Motivation

The main idea to create chain components to be able to build analogies registries like nsmd-k8s via SDK and be not dependent on Kubernetes staff.

Memory registries could be useful for the next goals:

1. Testing cmd application which depends on nsmd-k8s. For example cmd-nsmgr.
2. Building nsm registries application which not depend on Kubernetes.

Registry: handle case where NetworkServiceEndpoint is already set

Description

Need to fix TODO comment: https://github.com/networkservicemesh/sdk/blob/master/pkg/networkservice/common/discover/server.go#L46

Create authorization registry chain elements.

This is a followup for after #45.

We will also need authorization chain elements for connection.MonitorNetworkServiceServer and connection.MonitorNetworkServiceClient.

Please put them in the pkg/registry/common/ subdir

Registry: Add cache chain elements

Description

We need to migrate our caches components from monorepo to SDK. Should be created caches components for next registry models:

• registry.NetworkServiceEndpoint
• registry.NetworkService

NOTE: these components should not depend on k8s.

Use case

1. The user calls method Find on his client with the query.
2. If the cache has querying entry then it returns entry as a response.

Registry caches should be based on LRU caches to minimalize resource usage on the client-side.

Create authorization networkservice chain elements

This is a carry over of:
networkservicemesh/networkservicemesh#2032

and is already being worked on in #40

Create monitor core chain elements

Create in pkg/connection/core/ the analog: next, chain, adapters and trace chain elements for MonitorConnection{Server,Client}.

See pkg/*/core for examples for other APIs.

Interdomain: add registry chain element to proxying registration/find/unregistration to next registry

Context

#235

Description

We need to add possible to registries proxy all incoming registrations/unregistratiom/find to the next registry.

Use-case:

1. The registry receives registration with a special symbol in the name "@" or special label in labels.
2. The registry just proxying this traffic to the next registry.

Questions:

Do we want to use special symbol "@" in ns/nse name to mark it as interdomain? As an alternative variant, we can add special labels for nse. For example:

nse:
  name:
  labels:
   icmp-responder:
     k:v
   interdomain:
     k:v

Create authorization monitor chain elements

This is a followup for after #45.

We will also need authorization chain elements for connection.MonitorNetworkServiceServer and connection.MonitorNetworkServiceClient.

Please put them in the pkg/connection/common subdir (as they are part of the connection API).

Add coverage check on CI

Motivation

The coverage check might help to review PRs with tests.

Registry: add refresh chain element

Description

refresh - a chain client element that refreshes registration at 1/3 of the time between initial registration and expiration

Investigate race condition in eventchannel

https://github.com/networkservicemesh/sdk/pull/195/checks?check_run_id=584264737

Registry: add join chain element

Description

Registry join chain element will add a possible to aggregate a few registry clients to one. It will be useful for cmd-nsmgr. For example, we can join memory registry and real registry client.

Expand OPA into several pieces

The current authorization chain element provides only policy handling but not building. We need to expand the OPA mechanism into several pieces:

1. Builder mechanism which builds the provided policy (e.g. from yaml config file or string)
2. Handler mechanism which handles the building policies.

For example in pseudocode it might look like:

b := NewBuilder(configFile, elseOption...)
handler := b.Validate() //building the provided policy
ok := handler.eval(opaInputObj) //get the policy decision 

Add support for requesting a remote HTTP OPA micro service to perform the evaluation.

Support for embedding OPA was added with #40. Supporting a remote OPA evaluation engine will be useful.

Add benchmark testing on CI

We need to add benchmark testing on CI. We already have benchmark tests but they not run on CI: pkg/tools/serialize/serialize_test.go

Create DNSContext NetworkServiceServer chain elements

Please create in pkg/networkservice/connectioncontext/dnscontext/ a NetworkServiceServer chain element appropriate for setting a DNSContext for an Endpoint.

Improve performance of chain of adapters + next

Description

Chain of next + adapters elements can produce extra adapters per each of the chain elements.

For example:
If we have a chain of

server->adapted client->server->server

then it can be represented on call Request/Register/Close:

server->adapted client->adapted server->adapted server

We need to investigate and fix extra adaptation.

Interdomain: add registry connect chain element

Context

#235

Description

Proxy registry should be able to connect to the registry from another domain by the domain name.

Solution

Add analog of chain element connect for registries.

Example of usage:

1. Proxy registry receives query with network service name = [email protected].
2. Proxy registry resolves domain my.domain to IP.
3. Proxy registry connects to the next registry by IP from step2.

Create NetworkServiceClient chain element to add podName/nodeName/clusterName labels to request

Create NetworkServiceClient chain element to add podName/nodeName/clusterName labels to request.

Put it in sdk/pkg/networkservice/common/

Stabilize heal tests

Problem statement

After the latest merges, we faced new unstable heal tests:

TestHealClient_Request
TestNewClient_MissingConnectionsInInit

Logs

=== Failed
=== FAIL: pkg/networkservice/common/heal TestHealClient_Request (0.07s)
    client_test.go:116: 
        	Error Trace:	client_test.go:116
        	Error:      	Condition never satisfied
        	Test:       	TestHealClient_Request

=== FAIL: pkg/networkservice/common/heal TestNewClient_MissingConnectionsInInit (0.12s)
    client_test.go:217: 
        	Error Trace:	client_test.go:217
        	Error:      	Condition never satisfied
        	Test:       	TestNewClient_MissingConnectionsInInit

Link to CI job

https://github.com/networkservicemesh/sdk/runs/854630760?check_suite_focus=true

pkg/core/adapters can not be used with pkg/next

Actual behavior

pkg/core/adapters can adapt the server to client or vice versa, but if a server uses pkg/next then the adapters will not correctly adapt the server to client and it will panic on called Request/Close

Expected behavior

pkg/core/adapters can be used with pkg/next.

Steps to reproduce

1. Adapt server which uses next.Server(...) to client or vice versa
2. Try to call client.Request() or client.Close()`

Code example:
server.go

type configServer struct{}

// NewServer - inserts a vppagent *configurator.Config into the GRPC call context.Context
func NewServer() networkservice.NetworkServiceServer {
	return &configServer{}
}

func (c *configServer) Request(ctx context.Context, request *networkservice.NetworkServiceRequest) (*connection.Connection, error) {
	return next.Server(ctx).Request(ctx, request)
}

func (c *configServer) Close(ctx context.Context, conn *connection.Connection) (*empty.Empty, error) {
	return next.Server(ctx).Close(ctx, conn)
}

client_test.go

func TestClientBasic(t *testing.T) {
	next.NewNetworkServiceClient(NewServerToClient(&configServer{})).Close(nil, nil, nil)
}

Example from sdk-vppagent:
https://github.com/networkservicemesh/sdk-vppagent/blob/master/pkg/networkservice/vppagent/client.go

Stacktrace

panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x9c1110]

goroutine 6 [running]:
testing.tRunner.func1(0xc000140100)
	/usr/local/go/src/testing/testing.go:874 +0x60f
panic(0xa48b00, 0xeba540)
	/usr/local/go/src/runtime/panic.go:679 +0x1e0
github.com/networkservicemesh/sdk/pkg/networkservice/core/adapters.testServer.Close(0xb81160, 0xc0001344b0, 0x0, 0x0, 0x0, 0x0)
	/home/user/go/src/github.com/networkservicemesh/sdk/pkg/networkservice/core/adapters/client_test.go:21 +0x80
github.com/networkservicemesh/sdk/pkg/networkservice/core/adapters.(*serverToClient).Close(0xc00007b910, 0xb81160, 0xc0001344b0, 0x0, 0xc00007b930, 0x1, 0x1, 0x0, 0x0, 0x0)
	/home/user/go/src/github.com/networkservicemesh/sdk/pkg/networkservice/core/adapters/server_to_client.go:44 +0x8a
github.com/networkservicemesh/sdk/pkg/networkservice/core/next.(*nextClient).Close(0xc00000eb20, 0x0, 0x0, 0x0, 0xc00007b930, 0x1, 0x1, 0x0, 0x0, 0x0)
	/home/user/go/src/github.com/networkservicemesh/sdk/pkg/networkservice/core/next/client.go:67 +0x369
github.com/networkservicemesh/sdk/pkg/networkservice/core/adapters.TestClientBasic(0xc000140100)
	/home/user/go/src/github.com/networkservicemesh/sdk/pkg/networkservice/core/adapters/client_test.go:25 +0x1c0
testing.tRunner(0xc000140100, 0xb054d8)
	/usr/local/go/src/testing/testing.go:909 +0x13c
created by testing.(*T).Run
	/usr/local/go/src/testing/testing.go:960 +0x64f

next::Client doesn't work with chained elements

Motivation

When we pass the chained client element into next.NewNetworkServiceClient(), unexpected behaviour may occur in next.Request -- it will be impossible to make request through all the clients in the chain.