dapr Dapr Client API: a unreasonble design for Shutdown method

SDK API design:

Dapr Client API:

// Client is the interface for Dapr client implementation.
type Client interface {
	// InvokeBinding invokes specific operation on the configured Dapr binding.
	// This method covers input, output, and bi-directional bindings.
	InvokeBinding(ctx context.Context, in *InvokeBindingRequest) (out *BindingEvent, err error)

	// Shutdown the sidecar.
	Shutdown(ctx context.Context) error

	// WithTraceID adds existing trace ID to the outgoing context.
	WithTraceID(ctx context.Context, id string) context.Context

	// WithAuthToken sets Dapr API token on the instantiated client.
	WithAuthToken(token string)

	// Close cleans up all resources created by the client.
	Close()

        ......

	// GrpcClient returns the base grpc client if grpc is used and nil otherwise
	GrpcClient() pb.DaprClient
}

Dapr Service API

// Service represents Dapr callback service.
type Service interface {
        // AddServiceInvocationHandler appends provided service invocation handler with its name to the service.
        AddServiceInvocationHandler(name string, fn ServiceInvocationHandler) error
        // AddTopicEventHandler appends provided event handler with its topic and optional metadata to the service.
        // Note, retries are only considered when there is an error. Lack of error is considered as a success
        AddTopicEventHandler(sub *Subscription, fn TopicEventHandler) error
        // AddBindingInvocationHandler appends provided binding invocation handler with its name to the service.
        AddBindingInvocationHandler(name string, fn BindingInvocationHandler) error
        // RegisterActorImplFactory Register a new actor to actor runtime of go sdk
        RegisterActorImplFactory(f actor.Factory, opts ...config.Option)
        // Start starts service.
        Start() error
        // Stop stops the previously started service.
        Stop() error
        // Gracefully stops the previous started service
        GracefulStop() error
}

Problems

There are several parts to the incoming traffic of Dapr Sidecar:

Non-Dapr ecological external service traffic;
The outgoing traffic of the local MicroLogic;
Dapr standardizes the InputBinding of component capabilities and the subscribe message of pubsub;
RPC requests from other Dapr Sidecars;

For the first and fourth points above, it is primarily served by the Dapr Client API, that is:

For the first point, external services can establish a connection with a Dapr Sidecar of the remote POD (Dapr Sidecar+MicroLogic) through the Dapr Client API(the remote address and port, such as a dapr sidecar service vip ), and then access all services in the Dapr cluster, including standardized component capabilities and RPC capabilities;
For the fourth point, MicroLogic accesses the local Dapr Sidecar through the Dapr Client API, and then accesses standardized component capabilities and RPC capabilities;

For the Dapr Client API, there will be a method: Shutdown, which is used to close the Dapr Sidecar that appears in MicroLogic's local pair, that is, when the MicroLogic Client actively closes Shutdown first, stops the Dapr Sidecar from providing services to the outside world, and then passes the Close method. Close the connection between MicroLogic and Dapr Sidecar;

But there is a big problem here. When the non-local MicroLogic external service establishes a connection with the Dapr Sidecar through the remote address port, it seems that the Shutdown method of the Dapr Client API can be called, which directly closes the remote POD ( Dapr Sidecar+MicroLogic), this is too dangerous.

So here I think it exists some unreasonable designs:

For business services using Dapr Client API, it is not necessary to provide services externally through Dapr Sidecar;
For Dapr Service API, when MicroLogic provides external services, the external traffic can be routed into the local MicroLogic through Dapr Sidecar, and the traffic will be processed at the same time. When MicroLogic does not want to provide external services, the Dapr Sidecar should be prevented from receiving traffic and forwarded to the closed MicroLogic service. Therefore, when MicroLogic does not want to provide external services, it should first Shutdown the Dapr Sidecar to reduce traffic loss, and then close the connection between the Dapr Sidecar and MicroLogic.

Solutions:

solution 1:

The Shutdown channel of MicroLogic and Dapr Sidecar requires a specific token to complete the operation, and the token cannot be leaked to the outside world

solution 2:

Hide the Shutdown method. The Shutdown method should be placed in the Dapr Service API, not the Dapr Client API. Or we may delete Shutdown method in Dapr Client API, and be added to Stop/GracefulStop methods in Dapr Service API. When MicroLogic actively closes the connection, firstly create a client connection between MicroLogic and Dapr Sidecar, and then use the Shutdown method of DaprClient API to close the Dapr Sidecar service.

/cc @yaron2 @daixiang0 @msfussell @artursouza

May 19 '22 14:05 1046102779

@1046102779 does this solve the issue? https://docs.dapr.io/operations/security/api-token/

May 24 '22 21:05 yaron2

@1046102779 does this solve the issue? https://docs.dapr.io/operations/security/api-token/

I'm going to try it, but I think the second solution is better.

May 25 '22 01:05 1046102779

No matter 'Shutdown' or proposed 'GracefulShutdown', it is too dangerous to deploy dapr apps in production env if they are not secured, and I believe that's where api-token kicks in as @yaron2 points out.

And this issue cannot be solved by removing the corresponding admin commands from client SDK, since these APIs can be accessed without SDK.

I think the idea of 'graceful shutdown' is necessary for daprd, @yaron2 your opinion on this?

Jun 01 '22 06:06 beiwei30

This issue has been automatically marked as stale because it has not had activity in the last 60 days. It will be closed in the next 7 days unless it is tagged (pinned, good first issue, help wanted or triaged/resolved) or other activity occurs. Thank you for your contributions.

Jul 31 '22 06:07 dapr-bot

This issue has been automatically marked as stale because it has not had activity in the last 60 days. It will be closed in the next 7 days unless it is tagged (pinned, good first issue, help wanted or triaged/resolved) or other activity occurs. Thank you for your contributions.

Sep 29 '22 06:09 dapr-bot

This issue has been automatically closed because it has not had activity in the last 67 days. If this issue is still valid, please ping a maintainer and ask them to label it as pinned, good first issue, help wanted or triaged/resolved. Thank you for your contributions.

Oct 06 '22 06:10 dapr-bot