Support actortemplate secret env#20
Conversation
Benjamin Elder (BenTheElder)
added
the
feature
|
Reading PR #20 alongside Taahir Ahmed (@ahmedtd)'s #10/#51 (image-pinning admission policy) and Tim Hockin (@thockin)'s #18 (namespace design), there's a coherent "production-ready ActorTemplate" thread across the three surfaces that may be worth surfacing while TODO 2 here is still open.
The caching question (TODO 1) interacts: an apiserver client at atelet sees only its actor's namespace and is naturally bounded; a centralized cache in the manager has to handle namespace fan-in plus tenant invalidation. No strong opinion on which side wins, but #18's resolution may want to land first or co-evolve since it constrains TODO 2's design space. (Alex Bulankou (@AlexBulankou) asked me to take a closer look here.) |
…-secret-env # Conflicts: # cmd/ateapi/internal/controlapi/service.go # cmd/ateapi/internal/controlapi/workload_spec.go # cmd/ateapi/internal/controlapi/workload_spec_test.go # demos/claude-code-multiplex/README.md
Taahir Ahmed (ahmedtd)
left a comment
Taahir Ahmed (ahmedtd)
left a comment
There was a problem hiding this comment.
Can you also refactor the API type so we no longer use the K8s environment variable type, but instead one defined alongside ActorTemplate? It can still have the same shape as the k8s type.
| verbs: ["get", "watch", "list"] | ||
| # TODO: Decide whether env source resolution should keep using ate-api's | ||
| # broad read access or move to a more scoped delegation/admission model. | ||
| - apiGroups: [""] |
There was a problem hiding this comment.
We should not give a wide grant like this to ate-api-server by default. Instead, each demo should just grant ate-api-server access to the configmaps and secrets it needs.
| workloadSpec := &ateletpb.WorkloadSpec{ | ||
| PauseImage: actorTemplate.Spec.PauseImage, | ||
| } | ||
| resolver := envResolver{ |
There was a problem hiding this comment.
We are pessimizing Actor resumption by putting one or more k8s reads in the hot path. We need to move these off the hot path, maybe by using a shared cache of secrets and configmaps with a 1-minute expiry.
That way, only the first resumption is slowed down. And in the future, we could proactively fill and refresh the cache by watching ActorTemplates.
There was a problem hiding this comment.
In the future, once some sort of Kubernetes RBAC support for conditional authorization lands, we could migrate to letting ate-api-server read (and inform on) any configmap or secret with a specific label. That way, we could always just read the secret/configmap from the informer cache.
| return nil, err | ||
| } | ||
| if ateletEnv != nil { | ||
| ateletCtr.Env = append(ateletCtr.Env, ateletEnv) |
There was a problem hiding this comment.
Our RPC interceptor in atelet and ateom log the request proto --- we need to ensure that secrets are not written to logs.
3 participants
A couple of TODOs/Discussions which I think are important to have based on these changes.
Fixes #15