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

Create a route to terminate a dapr sidecar #1439

Closed
gafeol opened this issue Apr 23, 2020 · 7 comments
Closed

Create a route to terminate a dapr sidecar #1439

gafeol opened this issue Apr 23, 2020 · 7 comments

Comments

@gafeol
Copy link

gafeol commented Apr 23, 2020

In what area(s)?

/area operator

Describe the feature

Supporting a request that will allow the termination of sidecars, like the POST on /quitquitquit for Istio sidecars.

This would be useful for CronJobs, for instance:
Without a sidecar, a Pod for a CronJob would reach the state "Completed" after the task was done. A pod with a sidecar, on the other hand, never reaches the "Completed" state, because the sidecar is not terminated at the same time as the original task is completed. This causes the CronJob Pod to remain permanently on a "Running" state.
This is specially bad if one is using the "Forbid" concurrencyPolicy for Cronjobs, since a CronJob would not be able to run a second time if it has never finished in the first place.

If I'm not mistaken, there’s a way to kill Istio sidecars by sending a POST request to /quitquitquit, so I was wondering if there could be a similar feature on Dapr.

I found related issues to this problem on kubernetes and on istio.

@yaron2
Copy link
Member

yaron2 commented Apr 23, 2020

@gafeol do you think this poses a security risk where malicious code can shut down the Dapr container?

@gafeol
Copy link
Author

gafeol commented Apr 24, 2020

I believe you're right, having such an endpoint would allow any code inside container to disable the sidecar.
Still, there should be a way to terminate sidecars when the main container terminates, so that CronJobs can successfully restart.
Would you have any suggestions on how to solve this?

@jjcollinge
Copy link
Contributor

jjcollinge commented Apr 29, 2020

One idea, you could use a similar mechanism to https://github.com/karlkfi/kubexit, where the sidecar injector wraps the "app" process as a "supervisor" and writes a terminal status (tombstone) to a shared volume or dapr endpoint directly with the app's terminal status. Once dapr has detected this terminal status, it can gracefully shuts down if appropriate. You would limit this to pods in cron[jobs].

@jjcollinge
Copy link
Contributor

jjcollinge commented Apr 29, 2020

I guess the above tombstoning approach could also be susceptible to malicious code too. At a basic level, you could even just send signals (SIGTERM) from the supervisor to the app but that's not particularly secure either.

I think it's probably best to explore trying to use container lifecycle hooks such as preStop and a dapr endpoint with some level of authentication (maybe just a shared secret generated during deployment). That way only the hook should be able to terminate the sidecar.

@jjcollinge
Copy link
Contributor

#1030 would enable us to have an authenticated kill endpoint. It would have to assume the token isn't leaked by the the app though.

@artursouza
Copy link
Member

Since we have Dapr API token now, this endpoint should be safe. A reminder, it should work on HTTP, GRPC and from every SDK.

@artursouza
Copy link
Member

This was closed as part of the work done in #2689

@artursouza artursouza modified the milestones: v1.2, v1.1 May 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants