temporalio / proposals

Temporal proposals
https://temporal.io
MIT License
65 stars 18 forks source link

Admission Controllers #48

Open dtornow opened 2 years ago

dtornow commented 2 years ago

Author: Dominik Tornow

Summary of the feature being proposed

1. Admission Controllers

Provide Temporal Admission Controllers, similar to Kubernetes Admission Controllers, that intercept requests to Temporal before they get processed

Admission Controllers come in two flavors:

Validating Admission Controllers may not alter a request, Mutating Admission Controller may alter a request.

2. Admission Workflows & Activities

Enable developers to implement Admission Controllers as Temporal Workflows and Temporal Activities, therefore enabling the developer to extend Temporal with Temporal.

What value does this feature bring to Temporal?

Admission Controllers have proven to be a surprisingly simple yet surprisingly powerful extension mechanism for Kubernetes, they may prove to be a simple yet powerful extension mechanism for Temporal

  1. Example, Validating Admission Controller

The user may define a Validating Admission Controller that rejects a StartWorkflowExecutionRequest if the workflow_id does not match some regex

  1. Example, Mutating Admission Controller

The user may define a Mutating Admission Controller that implements a simple versioning strategy:

Are you willing to implement this feature yourself?

The temporal team

cretz commented 2 years ago

Can you clarify whether you are encouraging webhooks? Anyone can of course put a gRPC proxy in front or https://pkg.go.dev/go.temporal.io/server/temporal#WithAuthorizer and start Temporal programmatically. See https://docs.temporal.io/docs/server/security/#authorizer-plugin-interface. Do we need yet another mechanism? Do we need to support mutating or maybe allow gRPC interceptors directly?

dtornow commented 2 years ago
yiminc commented 2 years ago

Custom gRPC interceptor is already supported: https://github.com/temporalio/temporal/pull/2156 @cretz

robzienert commented 2 years ago

How would these admission controller workflows be registered? Would a worker startup and signal Temporal itself that it is available? Would these workflows be invoked for all start requests, or would the registration process include filtering criteria?

dtornow commented 2 years ago

While there is a wide range of possibilities, I would advocate for: Admission Controller Workflows are registered with the Temporal Cluster, probably per (namespace, workflow type, workflow queue) and (namespace, activity type, activity queue) with wildcard support and Temporal starts the Admission Controller Workflow - which is itself not subject to Admission Controllers (duh, haha)

cretz commented 2 years ago

I am not sure workflows are a good primitive for intercepting every RPC request due to performance issues. I think webhooks are reasonable. I don't think the benefit of dogfooding here (very minimal surely) outweighs the cost.

dtornow commented 2 years ago

Absolutely possible that workflows are not the right primitive here! Could be webhooks, could be WebAssembly hooks, could be all three.

jlegrone commented 2 years ago

This sounds really interesting! I can provide some context on how my company might leverage this feature internally.

Re https://github.com/temporalio/proposals/issues/48#issuecomment-994815587, we use the temporal.WithAuthorizer option in production today. Our authorizer implementation is both "validating" and "mutating" in that it:

  1. Extracts auth claims from the grpc headers (we probably don't want to pass grpc headers to admission controllers, so this would likely remain the same)
  2. Modifies the workflow or activity headers to include the auth claims
  3. Passes auth claims and request payloads to a rego policy engine (at this point the request may be rejected)

We've also considered performing other request modifications here, like rewriting task queues or aliasing workflow types.

I think there are a few things we've observed with this setup that this proposal might help improve upon:

Other things we care about: