Repository navigation
Configure initial egress policies for actors and goldens - #2159
Eitan Yarmush (EItanya) wants to merge 9 commits into
Conversation
| // Initial egress policy, copied atomically when an Actor is created from | ||
| // this template, including the golden Actor used for initialization. The | ||
| // policy is named "default" in the Actor's atespace, with its own metadata. | ||
| // Callers may update or delete the copy independently; changing the Actor's | ||
| // template does not update its policy. If absent, no policy is created; | ||
| // an empty block creates a policy with no rules. | ||
| // | ||
| // +k8s:optional | ||
| EgressPolicyTemplate default_egress_policy = 9; |
There was a problem hiding this comment.
I opted to create another message here for simplicity, but we could opt to instead require the whole EgressPolicy object with metadata and require the name to be default etc.
Julian Gutierrez Oschmann (@juli4n) and Tim Hockin (@thockin) for API context
|
Clarify for me - does this mean that every actor created from such a template has the same policy unless subsequently changed? Maybe that should be opt-in? E.g. Does this also mean that a subsequent call to set policy will require a read-modify-write? That may be surprising. |
Yes it would. However, I think that if you are creating from an
Now that I've spelled that out for you, I actually think these should be separate. The field I created here should really be something like Does that make sense?
Yes, and this is a very good point. I think that can mostly be solved with documentation, but it's definitely an interaction worth calling out. This is also a result of not having multiple policy layers, and therefore needing one policy source of truth for an actor. |
Eitan Yarmush (EItanya)
changed the title
Lior Lieberman (LiorLieberman)
left a comment
Lior Lieberman (LiorLieberman)
left a comment
There was a problem hiding this comment.
I am worried on just "goldenEgressPolicy". There are going to be many usecases that you need to just fetch github repos/s3/gcs for some assets. Not necessrially part of "golden" - how do we get those to work.
maybe its an "init actor" support and a policy for that? A very common use case for that is "I want an actor with no network, but give it all this github repos available in local folders (e.g for RL)
FWIW init actor can be useful for many other things.
If it's not part of the golden then it's already supported. When an actor comes up from golden it will already have its own identity. I specifically narrowed the scope of this PR to just handle the golden identity. Adding policy for pre readyz should already work.
That would be exactly what this would enable. The actor identity would not be allowed to make network calls, only during golden preparation. Mounting other data can/should be done with volumes otherwise.
|
f5b31fb to
cc5a23a
Compare
7199307 to
960abcc
Compare
| // Save the data in the storage layer. | ||
| stored, err := s.store.CreateActor(ctx, outActor) | ||
| var policy *ateapipb.EgressPolicy | ||
| if initial := template.GetGoldenEgressPolicy(); initial != nil && atespace == resources.GoldenActorAtespace { |
There was a problem hiding this comment.
Shouldn't this policy only be used when creating a golden for the template, rather than every CreateActor() ? IIUC this will use the golden policy for every derived actor, which is exactly what we said it was NOT?
There was a problem hiding this comment.
This is a leftover, I agree with you. However, this and the comment below do basically land us at changing the RPC to accept an optional initial policy, which could be used internally for goldens, and externally to use a single call to create the Actor. Overall I think the UX would be improved there anyway.
There was a problem hiding this comment.
This is especially relevant given that we're moving goldens out of this magical atespace
| storageClassLister storagev1listers.StorageClassLister | ||
| } | ||
|
|
||
| var _ store.Interface = (*ServiceImpl)(nil) |
There was a problem hiding this comment.
I know Julian Gutierrez Oschmann (@juli4n) thinking about layers
There was a problem hiding this comment.
As I said above, going to change the RPC and see how we feel about that
Eitan Yarmush (EItanya)
changed the title
2555fa5 to
8f23f53
Compare
Bowei Du (bowei)
added
area/api
Copy an optional template policy when creating an actor, including the golden actor, and write the actor and policy in one transaction. Preserve the distinction between an absent default and an explicitly empty policy, and let each actor policy evolve independently. Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
Expose golden_egress_policy for network access during golden initialization. Create its policy atomically with the golden actor while leaving ordinary actors, including golden snapshot restores, without an inherited runtime policy. Preserve absent and explicitly empty policies. Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
Reject a configured golden policy with no rules, since it grants no access beyond omitting the policy. Keep the policy object for future fields. Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
Pass the required policy name when reading actor egress policies so the tests follow the updated singleton subresource API. Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
Follow the public API proto initialism naming so golden egress tests compile against the current generated types. Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
Have the template reconciler explicitly supply the golden policy through CreateActor instead of inferring golden intent from the atespace. Validate and authorize optional initial policies and create them atomically with their actors. Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
8f23f53 to
70da2cd
Compare
Require the GoldenEgressPolicy server gate and client preview opt-in when creating templates with a golden egress policy. Keep existing templates usable after the gate is disabled and leave explicit CreateActor policies available without preview. Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
Use the existing policy resource and metadata validation for atomic actor creation. Materialize golden policy templates in the reconciler so the RPC and storage interfaces share the same policy type. Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
CreateActor deliberately accepts a second resource so its egress policy can be created in the same transaction. Record that exception to the single-resource create request convention. Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
4 participants
Golden initialization can need network access that runtime actors should not inherit. This adds
ActorTemplate.goldenEgressPolicyand optionalCreateActorRequest.initial_egress_policy. The template reconciler explicitly passes the golden policy through the publicCreateActorAPI, so actor creation does not infer golden intent from the atespace.The template field requires the
GoldenEgressPolicypreview gate on the server and client opt-in throughkubectl ate --previewor theAte-Preview-Allowed: trueheader. Disabling the gate blocks new templates that set the field; existing templates keep their configuration and can still finish golden initialization or be deleted.CreateActorRequest.initial_egress_policydoes not require preview opt-in.The actor and its initial policy are created in one transaction. Supplying a policy requires permission to create an actor egress policy in addition to the existing actor and template permissions. The RPC accepts a full
EgressPolicy, withmetadata.name: defaultandmetadata.atespacematching the actor; the server assigns UID, version, and timestamps. The template keeps a metadata-freeEgressPolicyTemplatewrapper, which the reconciler turns into a full policy for the RPC. Both require at least one rule and use the existing rule defaults and validation. Omitting the request field creates no policy, including when restoring from a golden snapshot or tag. The resulting policy is managed through the existing Actor egress policy RPCs.Related to #1324. Motivation and API rationale: #2358.
tools/apitoolpass with-race; all repository verifiers pass. The root-module race run hit a CA reload failure infake-ateletand color-output failures ininternal/e2e, both reproduced on upstream/main. The color tests pass withNO_COLORunset. CI is rerunning after adding the required request-shape exemption for atomic actor/policy creation.