diff --git a/docs/architecture/configuration-and-compilation.md b/docs/architecture/configuration-and-compilation.md index af29627749..305c8e74cc 100644 --- a/docs/architecture/configuration-and-compilation.md +++ b/docs/architecture/configuration-and-compilation.md @@ -216,9 +216,14 @@ millicores and memory bytes for every harness, rounding positive fractions up to the next unit. Standalone sandbox limits remain under `sandboxes` configuration. Harness and SandboxTemplate snapshot locations require Substrate's -`PerTemplateSnapshotLocation` preview gate. The CLI installer enables it; direct -Substrate installations must include it in their Helm `preview` list. The +`PerTemplateSnapshotLocation` preview gate, and the golden actor's egress policy +requires `GoldenEgressPolicy`. The CLI installer enables both; direct Substrate +installations must include them in their Helm `preview` list. The Substrate client opts ActorTemplate creation requests into preview features. +Without `GoldenEgressPolicy`, Substrate rejects ActorTemplates for Agents with +git or S3 skill sources. The Agent reports `ActorTemplateRejected` with a message +that names the gate. The controller retries with backoff, so the Agent becomes +Ready after the gate is turned on. A missing WorkerPool reports `WorkerPoolNotFound`; an unsupported class reports `RevisionInvalid`. Neither produces a desired ActorTemplate. WorkerPool updates @@ -308,9 +313,12 @@ download cache or fall back to an in-runtime OCI pull. Package limits and path/symlink validation still apply. Build skill images from scratch with just package contents; arbitrary ORAS artifact layouts are not -supported. Existing Git and S3 fetches remain runtime operations and retain -their golden-runtime networking limitations. The Python ADK does not yet consume -the `agent_plugins` runtime contract. BYO images must consume that contract to +supported. The runtime fetches git and S3 sources when the golden actor starts. +The ActorTemplate's golden egress policy allows only the origins (scheme, host, +and port) of those sources, for the root agent and its shared agents. Sessions +restore the fetched sources from the golden snapshot and reach those origins +through their own egress policy. The Python ADK does not yet consume the +`agent_plugins` runtime contract. BYO images must consume that contract to load these skills; the Go ADK BYO fixture does so. ### Runtime configuration diff --git a/go/core/internal/controller/preparation_test.go b/go/core/internal/controller/preparation_test.go index 22f7ab178a..d7cab4ae16 100644 --- a/go/core/internal/controller/preparation_test.go +++ b/go/core/internal/controller/preparation_test.go @@ -22,6 +22,7 @@ import ( corev1 "k8s.io/api/core/v1" apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/validation/field" kagentfake "github.com/kagent-dev/kagent/go/api/clientset/versioned/fake" apiv1 "github.com/kagent-dev/kagent/go/api/gen/kagent/api/v1" @@ -139,7 +140,7 @@ func TestUnresolvedPoolReleasesAbandonedRevision(t *testing.T) { } func TestPreparationErrorsPublishStatusAndRecover(t *testing.T) { - for _, test := range []struct { + tests := []struct { name string templates fakeActorTemplates store fakeRuntimeRevisionStore @@ -151,7 +152,9 @@ func TestPreparationErrorsPublishStatusAndRecover(t *testing.T) { {name: "atespace failed", templates: fakeActorTemplates{ensureErr: status.Error(codes.PermissionDenied, "private-credential")}, code: codes.PermissionDenied}, {name: "pair store failed", store: fakeRuntimeRevisionStore{pairErr: errors.New("private-database-connection")}, code: codes.Unknown}, {name: "revision store failed", store: fakeRuntimeRevisionStore{revisionErr: errors.New("private-database-connection")}, code: codes.Unknown}, - } { + } + for i := range tests { + test := &tests[i] t.Run(test.name, func(t *testing.T) { collections, _ := newPreparationTestCollections(t, "microvm") initial := collections.Reconciliations.List()[0] @@ -238,14 +241,16 @@ func TestPreparationRevisionIsolation(t *testing.T) { } func TestPreparationQueueAndPollerBackoff(t *testing.T) { - for _, test := range []struct { + tests := []struct { name string templates fakeActorTemplates store fakeRuntimeRevisionStore }{ {name: "revision being collected", store: fakeRuntimeRevisionStore{pairErr: database.ErrObjectDeleting}}, {name: "persistence unavailable", store: fakeRuntimeRevisionStore{revisionErr: errors.New("database unavailable")}}, - } { + } + for i := range tests { + test := &tests[i] t.Run(test.name, func(t *testing.T) { synctest.Test(t, func(t *testing.T) { collections, _ := newPreparationTestCollections(t, "microvm") @@ -486,6 +491,84 @@ func TestPreparationTerminalFailures(t *testing.T) { } } +// Substrate without the GoldenEgressPolicy preview gate rejects a template +// with a golden egress policy. Enabling the gate must heal the Agent. +func TestPreparationRetriesGoldenEgressRejection(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + collections, _ := newPreparationTestCollectionsFor(t, "microvm", gitSkillTemplate()) + initial := collections.Reconciliations.List()[0] + gateOff := field.ErrorList{field.Forbidden(field.NewPath("actor_template", "golden_egress_policy"), "")}.ToAggregate() + templates := &fakeActorTemplates{createErr: status.Error(codes.InvalidArgument, gateOff.Error())} + store := &fakeRuntimeRevisionStore{} + statusClient := kagentfake.NewSimpleClientset(initial.Agent.DeepCopy()).ApiV1alpha3() + r := newReconciler(collections, templates, store, statusClient) + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + go func() { assert.NoError(t, r.Start(ctx)) }() + synctest.Wait() + + failure := collections.Reconciliations.GetKey(initial.ResourceName()).PreparationFailure + require.NotNil(t, failure) + require.True(t, failure.Retryable, "enabling the gate must not need a controller restart") + published, err := statusClient.Agents(initial.Agent.Namespace).Get(t.Context(), initial.Agent.Name, metav1.GetOptions{}) + require.NoError(t, err) + ready := apimeta.FindStatusCondition(published.Status.Conditions, kagentv1alpha3.AgentConditionReady) + require.Equal(t, "ActorTemplateRejected", ready.Reason) + require.Contains(t, ready.Message, "GoldenEgressPolicy preview gate") + require.NotContains(t, ready.Message, "golden_egress_policy") + + time.Sleep(5 * time.Minute) + synctest.Wait() + require.Greater(t, store.upserts(), 2, "the queue retries the rejected template") + require.Less(t, store.upserts(), 30, "retries back off") + + templates.setCreateErr(nil) + time.Sleep(time.Minute) + synctest.Wait() + require.NotNil(t, templates.created(), "the template is created once Substrate accepts it") + require.Nil(t, collections.Reconciliations.GetKey(initial.ResourceName()).PreparationFailure) + cancel() + synctest.Wait() + }) +} + +// Other validation errors on a template with a golden egress policy stay +// terminal: enabling the gate cannot repair them. +func TestPreparationGoldenEgressOtherRejectionIsTerminal(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + collections, _ := newPreparationTestCollectionsFor(t, "microvm", gitSkillTemplate()) + initial := collections.Reconciliations.List()[0] + require.NotNil(t, initial.Target.ActorTemplate.GetGoldenEgressPolicy()) + templates := &fakeActorTemplates{createErr: status.Error(codes.InvalidArgument, "actor_template.containers[0].env: Too many")} + store := &fakeRuntimeRevisionStore{} + statusClient := kagentfake.NewSimpleClientset(initial.Agent.DeepCopy()).ApiV1alpha3() + r := newReconciler(collections, templates, store, statusClient) + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + go func() { assert.NoError(t, r.Start(ctx)) }() + synctest.Wait() + + failure := collections.Reconciliations.GetKey(initial.ResourceName()).PreparationFailure + require.NotNil(t, failure) + require.False(t, failure.Retryable) + require.Equal(t, "ActorTemplateRejected", failure.Reason) + require.NotContains(t, failure.Message, "GoldenEgressPolicy") + time.Sleep(5 * time.Minute) + synctest.Wait() + require.Equal(t, 1, store.upserts(), "a terminal rejection is not retried") + cancel() + synctest.Wait() + }) +} + +func gitSkillTemplate() *kagentv1alpha3.AgentTemplate { + return &kagentv1alpha3.AgentTemplate{ObjectMeta: metav1.ObjectMeta{Namespace: "team-a", Name: "assistant", UID: "template-uid"}, Spec: kagentv1alpha3.AgentTemplateSpec{ + Skills: []kagentv1alpha3.AgentTemplateSkill{{Name: "review", Source: kagentv1alpha3.ArtifactSource{Git: &kagentv1alpha3.GitArtifact{ + URL: "https://git.example.com/skills.git", Commit: strings.Repeat("a", 40), + }}}}, + }} +} + func TestInvalidActorTemplateHasNoCompiledTarget(t *testing.T) { collections, harnesses := newPreparationTestCollections(t, "microvm") initial := collections.Reconciliations.List()[0] diff --git a/go/core/internal/controller/reconciler.go b/go/core/internal/controller/reconciler.go index 2e652aeaf9..0d0fd8b8de 100644 --- a/go/core/internal/controller/reconciler.go +++ b/go/core/internal/controller/reconciler.go @@ -19,6 +19,7 @@ import ( apiequality "k8s.io/apimachinery/pkg/api/equality" apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/client-go/rest" "k8s.io/client-go/util/workqueue" @@ -404,6 +405,16 @@ func (r *Reconciler) reconcileAgent(ctx context.Context, key string) error { return r.observePreparationError(*state, fmt.Errorf("ensure Atespace %s: %w", desiredRef.GetAtespace(), err)) } observed, err = r.templates.CreateActorTemplate(ctx, target.ActorTemplate) + if status.Code(err) == codes.InvalidArgument && strings.Contains(status.Convert(err).Message(), goldenEgressGateOff) { + // The operator can enable the preview gate, so keep retrying with backoff. + r.observePreparation(*state, goldenProgress{}, &ReconciliationFailure{ + Condition: kagentv1alpha3.AgentConditionReady, + Reason: "ActorTemplateRejected", + Message: "Substrate rejected the ActorTemplate. Git and S3 skill sources need Substrate's GoldenEgressPolicy preview gate. Add GoldenEgressPolicy to the preview list in Substrate's Helm values.", + Retryable: true, + }) + return fmt.Errorf("create ActorTemplate %s/%s: %w", desiredRef.GetAtespace(), desiredRef.GetName(), err) + } if status.Code(err) == codes.InvalidArgument { // Validation rejects the immutable request, so retries cannot repair // it. Backend messages may quote configuration; keep status safe. @@ -461,6 +472,10 @@ func (r *Reconciler) reconcileAgent(ctx context.Context, key string) error { return nil } +// goldenEgressGateOff is how Substrate reports a golden egress policy while its +// GoldenEgressPolicy preview gate is off. Other validation errors are terminal. +var goldenEgressGateOff = field.Forbidden(field.NewPath("actor_template", "golden_egress_policy"), "").Error() + func (r *Reconciler) observePreparationError(state AgentReconciliation, err error) error { if state.Target == nil { return err diff --git a/go/core/internal/controller/reconciler_test.go b/go/core/internal/controller/reconciler_test.go index 523eb15417..3f3e3e0429 100644 --- a/go/core/internal/controller/reconciler_test.go +++ b/go/core/internal/controller/reconciler_test.go @@ -3,6 +3,7 @@ package controller import ( "context" "errors" + "sync" "sync/atomic" "testing" "testing/synctest" @@ -282,6 +283,9 @@ func (s *failingFinalizationStore) DeleteRuntimeRevision(ctx context.Context, re type fakeActorTemplates struct { actorEgressStub + // mu guards template and createErr for tests that change them while the + // reconciler's queue runs. + mu sync.Mutex template *ateapipb.ActorTemplate ensureErr error getErr error @@ -295,6 +299,8 @@ type fakeActorTemplates struct { func (f *fakeActorTemplates) EnsureAtespace(context.Context, string) error { return f.ensureErr } func (f *fakeActorTemplates) GetActorTemplate(context.Context, string, string) (*ateapipb.ActorTemplate, error) { + f.mu.Lock() + defer f.mu.Unlock() if f.getErr != nil { return nil, f.getErr } @@ -305,6 +311,8 @@ func (f *fakeActorTemplates) GetActorTemplate(context.Context, string, string) ( } func (f *fakeActorTemplates) CreateActorTemplate(_ context.Context, template *ateapipb.ActorTemplate) (*ateapipb.ActorTemplate, error) { + f.mu.Lock() + defer f.mu.Unlock() if f.createErr != nil { return nil, f.createErr } @@ -316,6 +324,18 @@ func (f *fakeActorTemplates) CreateActorTemplate(_ context.Context, template *at return f.template, nil } +func (f *fakeActorTemplates) setCreateErr(err error) { + f.mu.Lock() + defer f.mu.Unlock() + f.createErr = err +} + +func (f *fakeActorTemplates) created() *ateapipb.ActorTemplate { + f.mu.Lock() + defer f.mu.Unlock() + return f.template +} + func (f *fakeActorTemplates) DeleteActorTemplate(_ context.Context, _, _, uid string) error { f.deletedUIDs = append(f.deletedUIDs, uid) if f.deleteErr != nil { @@ -332,6 +352,9 @@ func (f *fakeActorTemplates) DeleteActorTemplate(_ context.Context, _, _, uid st type fakeRuntimeRevisionStore struct { egressTargetStoreStub + // mu guards pair and pairCalls for tests that read them while the + // reconciler's queue runs. + mu sync.Mutex pair *database.AgentDefinition revision *database.RuntimeRevision markedSuccessful bool @@ -374,11 +397,19 @@ func (s *fakeRuntimeRevisionStore) ListAgentEgressSources(context.Context) ([]da } func (s *fakeRuntimeRevisionStore) UpsertAgentDefinition(_ context.Context, pair database.AgentDefinition) error { + s.mu.Lock() + defer s.mu.Unlock() s.pairCalls++ s.pair = &pair return s.pairErr } +func (s *fakeRuntimeRevisionStore) upserts() int { + s.mu.Lock() + defer s.mu.Unlock() + return s.pairCalls +} + func (s *fakeRuntimeRevisionStore) RecordRuntimeRevision(_ context.Context, revision database.RuntimeRevision, options database.RecordRevisionOptions) (database.GoldenRetry, error) { s.recordCalls++ if s.revisionErr != nil { diff --git a/go/core/internal/substrate/actor_template.go b/go/core/internal/substrate/actor_template.go index a56e1cf17c..f27aac0aa8 100644 --- a/go/core/internal/substrate/actor_template.go +++ b/go/core/internal/substrate/actor_template.go @@ -91,6 +91,10 @@ func ActorTemplateForRevision(spec *translator.Revision, revisionID translator.R if err != nil { return nil, err } + goldenEgress, err := GoldenEgressPolicy(spec.GoldenEgressDestinations) + if err != nil { + return nil, fmt.Errorf("compile golden egress policy: %w", err) + } template := &ateapipb.ActorTemplate{ Metadata: &ateapipb.ResourceMetadata{Atespace: spec.Namespace, Name: name}, @@ -118,6 +122,7 @@ func ActorTemplateForRevision(spec *translator.Revision, revisionID translator.R // Pin the server default so immutable comparison survives creation. GoldenSnapshotConfig: &ateapipb.GoldenSnapshotConfig{Mode: ateapipb.GoldenSnapshotMode_GOLDEN_SNAPSHOT_MODE_ENABLED}, }, + GoldenEgressPolicy: goldenEgress, Volumes: []*ateapipb.Volume{ {Name: durableDataVolume, DurableDir: &ateapipb.DurableDirVolumeSource{}}, // Substrate regenerates this projection on Run and Restore. A fork @@ -173,11 +178,12 @@ func actorTemplateSpec(template *ateapipb.ActorTemplate) *ateapipb.ActorTemplate Atespace: template.GetMetadata().GetAtespace(), Name: template.GetMetadata().GetName(), }, - WorkerSelector: template.GetWorkerSelector(), - Containers: template.GetContainers(), - Volumes: template.GetVolumes(), - SnapshotConfig: template.GetSnapshotConfig(), - Resources: template.GetResources(), + WorkerSelector: template.GetWorkerSelector(), + Containers: template.GetContainers(), + Volumes: template.GetVolumes(), + SnapshotConfig: template.GetSnapshotConfig(), + Resources: template.GetResources(), + GoldenEgressPolicy: template.GetGoldenEgressPolicy(), } } diff --git a/go/core/internal/substrate/actor_template_test.go b/go/core/internal/substrate/actor_template_test.go index 5570410f34..665a70d461 100644 --- a/go/core/internal/substrate/actor_template_test.go +++ b/go/core/internal/substrate/actor_template_test.go @@ -181,6 +181,58 @@ func mustSpecDigest(t *testing.T, template *ateapipb.ActorTemplate) string { return digest } +// The golden actor fetches git and S3 skill sources before its snapshot. +// Its policy must allow exactly those origins and nothing else. +func TestActorTemplateGoldenEgressPolicy(t *testing.T) { + for _, test := range []struct { + name string + destinations []string + want *ateapipb.EgressPolicyTemplate + }{ + {name: "OCI only", want: nil}, + { + name: "git and S3 origins", + destinations: []string{"http://git.internal.example:8080", "https://git.example.com:443", "https://objects.example.com:9000"}, + want: &ateapipb.EgressPolicyTemplate{Rules: []*ateapipb.EgressRule{ + {Http: &ateapipb.HttpRule{Hostnames: []string{"git.internal.example"}, Ports: &ateapipb.Ports{Numbers: []int32{8080}}}}, + {Https: &ateapipb.HttpsRule{Hostnames: []string{"git.example.com"}, Ports: &ateapipb.Ports{Numbers: []int32{443}}}}, + {Https: &ateapipb.HttpsRule{Hostnames: []string{"objects.example.com"}, Ports: &ateapipb.Ports{Numbers: []int32{9000}}}}, + }}, + }, + } { + t.Run(test.name, func(t *testing.T) { + spec := &translator.Revision{CPU: "1", Memory: "1Gi", Namespace: "agents", AgentName: "helper", WorkerPoolName: "default", + AgentCard: &a2apb.AgentCard{Name: "helper", Version: "v1", Capabilities: &a2apb.AgentCapabilities{}, + SupportedInterfaces: []*a2apb.AgentInterface{{Url: "http://127.0.0.1:80", ProtocolBinding: "GRPC", ProtocolVersion: "1.0"}}, + DefaultInputModes: []string{"text"}, DefaultOutputModes: []string{"text"}}, + EgressDestinations: []string{"http://kagent-controller.kagent:8083", "https://api.openai.com:443"}, + GoldenEgressDestinations: test.destinations, + } + revisionID, err := spec.Digest() + require.NoError(t, err) + template, err := ActorTemplateForRevision(spec, revisionID) + require.NoError(t, err) + require.True(t, proto.Equal(test.want, template.GetGoldenEgressPolicy()), "golden egress policy = %v", template.GetGoldenEgressPolicy()) + }) + } +} + +func TestActorTemplateSpecEqualComparesGoldenEgressPolicy(t *testing.T) { + left := &ateapipb.ActorTemplate{ + Metadata: &ateapipb.ResourceMetadata{Atespace: "team-a", Name: "template"}, + Containers: []*ateapipb.Container{{Name: "agent", Image: "agent:v1"}}, + GoldenEgressPolicy: &ateapipb.EgressPolicyTemplate{Rules: []*ateapipb.EgressRule{ + {Https: &ateapipb.HttpsRule{Hostnames: []string{"git.example.com"}, Ports: &ateapipb.Ports{Numbers: []int32{443}}}}, + }}, + } + right := proto.CloneOf(left) + require.True(t, ActorTemplateSpecEqual(left, right)) + right.GoldenEgressPolicy.Rules[0].Https.Hostnames = []string{"objects.example.com"} + require.False(t, ActorTemplateSpecEqual(left, right), "a changed golden egress policy was accepted") + right.GoldenEgressPolicy = nil + require.False(t, ActorTemplateSpecEqual(left, right), "a removed golden egress policy was accepted") +} + func TestActorTemplateEnvironmentValueSize(t *testing.T) { const configOverhead = len(`{"instruction":""}`) for _, test := range []struct { diff --git a/go/core/internal/substrate/client.go b/go/core/internal/substrate/client.go index 8739fc57d2..ea698bfc84 100644 --- a/go/core/internal/substrate/client.go +++ b/go/core/internal/substrate/client.go @@ -149,8 +149,9 @@ func (c *Client) GetActorTemplate(ctx context.Context, atespace, name string) (* func (c *Client) CreateActorTemplate(ctx context.Context, template *ateapipb.ActorTemplate) (*ateapipb.ActorTemplate, error) { ctx, cancel := c.callCtx(ctx) defer cancel() - // Substrate requires request opt-in as well as the server's - // PerTemplateSnapshotLocation gate to honor the configured snapshot location. + // Substrate requires request opt-in as well as the server's preview gates: + // PerTemplateSnapshotLocation for the snapshot location and + // GoldenEgressPolicy for the golden actor's egress policy. ctx = metadata.AppendToOutgoingContext(ctx, "ate-preview-allowed", "true") return c.ControlClient.CreateActorTemplate(ctx, &ateapipb.CreateActorTemplateRequest{ActorTemplate: template}) } diff --git a/go/core/internal/substrate/policy.go b/go/core/internal/substrate/policy.go index beab8790e4..dfcc84b00f 100644 --- a/go/core/internal/substrate/policy.go +++ b/go/core/internal/substrate/policy.go @@ -52,20 +52,42 @@ func ActorEgressPolicy(atespace string, destinations []string, credentials []egr ActorJwt: &ateapipb.ActorJwtSource{Audiences: []string{RuntimeJWTAudience}, ExpirationSeconds: 300}, }) } - target, _ := egress.ParseOrigin(origin) // validated by CanonicalAccess - port, _ := strconv.ParseInt(target.Port(), 10, 32) - ports := &ateapipb.Ports{Numbers: []int32{int32(port)}} - // One rule per origin: exact matches win over wildcard rules, keeping - // credentials on their exact origin even when a wildcard also grants access. - if target.Scheme == "https" { - policy.Rules = append(policy.Rules, &ateapipb.EgressRule{Https: &ateapipb.HttpsRule{Hostnames: []string{target.Hostname()}, Ports: ports, Effects: effects[origin]}}) - } else { - policy.Rules = append(policy.Rules, &ateapipb.EgressRule{Http: &ateapipb.HttpRule{Hostnames: []string{target.Hostname()}, Ports: ports, Effects: effects[origin]}}) - } + policy.Rules = append(policy.Rules, originRule(origin, effects[origin])) } return policy, nil } +// GoldenEgressPolicy allows the golden actor to reach only the given origins +// while it starts. Actors restored from its snapshot use their own egress policy. +func GoldenEgressPolicy(destinations []string) (*ateapipb.EgressPolicyTemplate, error) { + if len(destinations) == 0 { + return nil, nil + } + access, err := egress.CanonicalAccess(destinations, nil, nil) + if err != nil { + return nil, err + } + policy := &ateapipb.EgressPolicyTemplate{} + for _, origin := range access.Destinations { + policy.Rules = append(policy.Rules, originRule(origin, nil)) + } + return policy, nil +} + +// originRule allows one origin. The origin must come from CanonicalAccess, +// which already rejected anything ParseOrigin cannot parse. One rule per +// origin: exact matches win over wildcard rules, keeping credentials on their +// exact origin even when a wildcard also grants access. +func originRule(origin string, effects *ateapipb.HttpRuleEffects) *ateapipb.EgressRule { + target, _ := egress.ParseOrigin(origin) + port, _ := strconv.ParseInt(target.Port(), 10, 32) + ports := &ateapipb.Ports{Numbers: []int32{int32(port)}} + if target.Scheme == "https" { + return &ateapipb.EgressRule{Https: &ateapipb.HttpsRule{Hostnames: []string{target.Hostname()}, Ports: ports, Effects: effects}} + } + return &ateapipb.EgressRule{Http: &ateapipb.HttpRule{Hostnames: []string{target.Hostname()}, Ports: ports, Effects: effects}} +} + // defaultEgressPolicyCompiler adds the allowlist, Secret credentials, and Actor JWTs. It runs first. type defaultEgressPolicyCompiler struct { access egress.Access diff --git a/go/core/internal/translator/compiler.go b/go/core/internal/translator/compiler.go index e8602b3bc9..0fde05ad67 100644 --- a/go/core/internal/translator/compiler.go +++ b/go/core/internal/translator/compiler.go @@ -181,9 +181,32 @@ func (c *Compiler) compileConfiguration(ctx context.Context, agent *v1alpha3.Age // Normalize once after the complete harness configuration has been compiled. slices.Sort(result.SkillImages) result.SkillImages = slices.Compact(result.SkillImages) + origins, err := goldenEgressOrigins(tree.Root) + if err != nil { + return nil, err + } + result.GoldenEgressDestinations = slices.Compact(slices.Sorted(slices.Values(origins))) return result, nil } +// goldenEgressOrigins returns the git and S3 origins of every skill source in +// the tree. The golden actor fetches them for the root and its shared agents. +func goldenEgressOrigins(agent *resolvedTemplate) ([]string, error) { + skills, err := CompileSkillResources(agent.Template) + if err != nil { + return nil, err + } + origins := skills.Egress + for _, shared := range agent.Shared { + child, err := goldenEgressOrigins(shared.Agent) + if err != nil { + return nil, err + } + origins = append(origins, child...) + } + return origins, nil +} + func harnessType(harness *HarnessConfiguration) HarnessType { switch { case harness.Spec.Kagent != nil: diff --git a/go/core/internal/translator/compiler_test.go b/go/core/internal/translator/compiler_test.go index caa5614f73..872a11d475 100644 --- a/go/core/internal/translator/compiler_test.go +++ b/go/core/internal/translator/compiler_test.go @@ -776,6 +776,9 @@ func TestCompileAgentSharedADKConfig(t *testing.T) { Skills: []v1alpha3.AgentTemplateSkill{{Name: "review", Source: v1alpha3.ArtifactSource{ OCI: "ghcr.io/acme/review@sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", }}}, + Plugins: []v1alpha3.PluginBundle{{Source: v1alpha3.ArtifactSource{Git: &v1alpha3.GitArtifact{ + URL: "https://git.research.example/plugin", Commit: "dddddddddddddddddddddddddddddddddddddddd", + }}}}, }, } root := &v1alpha3.AgentTemplate{ @@ -817,6 +820,7 @@ func TestCompileAgentSharedADKConfig(t *testing.T) { require.NotContains(t, revision.EgressDestinations, "https://tickets.example.com:443") require.Contains(t, revision.EgressDestinations, "https://search.example.com:443") require.NotContains(t, revision.EgressDestinations, "https://ghcr.io:443") + require.Equal(t, []string{"https://git.research.example:443", "https://github.com:443"}, revision.GoldenEgressDestinations, "shared agents fetch their sources in the same golden actor") require.Equal(t, []translator.SkillImage{translator.SkillImage(child.Spec.Skills[0].Source.OCI)}, revision.SkillImages) require.Equal(t, revision.SkillImages[0].MountPath(), config.Agent.SubAgents[0].AgentPlugins.Skills[0].Source.GetDirectory()) require.Equal(t, config.Agent.AgentPlugins.Skills[0].Source.GetDirectory(), config.Agent.SubAgents[0].AgentPlugins.Skills[0].Source.GetDirectory()) diff --git a/go/core/internal/translator/revision.go b/go/core/internal/translator/revision.go index 19c7792b57..8e41765d13 100644 --- a/go/core/internal/translator/revision.go +++ b/go/core/internal/translator/revision.go @@ -72,6 +72,11 @@ type Revision struct { TokenPassthroughTargets []egresspolicy.GatewayTarget // EgressDestinations is the HTTP(S) origin allowlist required by this revision. EgressDestinations []string + // GoldenEgressDestinations are the sorted git and S3 origins the runtime + // fetches skill sources from at startup, before the golden snapshot. + // They feed the golden actor's own policy. Sessions reach the same origins + // through EgressDestinations. + GoldenEgressDestinations []string } // Equals compares the Agent Card's contents without inspecting protobuf caches. @@ -112,7 +117,9 @@ func (r *Revision) Digest() (RevisionID, error) { Credentials []egress.Credential `json:"credentials,omitempty"` TokenPassthroughTargets []egresspolicy.GatewayTarget `json:"tokenPassthroughTargets,omitempty"` EgressDestinations []string `json:"egressDestinations"` - SandboxClass atev1.SandboxClass `json:"sandboxClass"` + // Omitted when empty so revisions without git or S3 sources keep their IDs. + GoldenEgressDestinations []string `json:"goldenEgressDestinations,omitempty"` + SandboxClass atev1.SandboxClass `json:"sandboxClass"` // Placement now matches the pool's namespace as well as its name. // Hash it explicitly so existing immutable templates do not conflict after upgrades. @@ -124,7 +131,8 @@ func (r *Revision) Digest() (RevisionID, error) { SkillImages: r.SkillImages, WorkerPoolName: r.WorkerPoolName, SnapshotLocation: r.SnapshotLocation, TokenPassthroughTargets: r.TokenPassthroughTargets, Credentials: r.Credentials, EgressDestinations: r.EgressDestinations, - SandboxClass: sandboxClass, WorkerPoolNamespace: r.Namespace, + GoldenEgressDestinations: r.GoldenEgressDestinations, + SandboxClass: sandboxClass, WorkerPoolNamespace: r.Namespace, }) if err != nil { return RevisionID{}, fmt.Errorf("marshal runtime revision inputs: %w", err) diff --git a/go/core/internal/translator/revision_test.go b/go/core/internal/translator/revision_test.go index bbfa88638e..5b3d909606 100644 --- a/go/core/internal/translator/revision_test.go +++ b/go/core/internal/translator/revision_test.go @@ -49,6 +49,18 @@ func TestRevisionDigestReplacesNameOnlyWorkerPoolTemplate(t *testing.T) { require.Equal(t, id, repeated) } +func TestRevisionDigestIncludesGoldenEgressDestinations(t *testing.T) { + digest := func(destinations []string) RevisionID { + revision := &Revision{Namespace: "agents", AgentName: "helper", EgressDestinations: []string{"https://git.example.com:443"}, GoldenEgressDestinations: destinations} + id, err := revision.Digest() + require.NoError(t, err) + return id + } + unset := digest(nil) + require.Equal(t, unset, digest([]string{}), "revisions without golden egress keep their existing ActorTemplates") + require.NotEqual(t, unset, digest([]string{"https://git.example.com:443"}), "the golden egress policy is part of the immutable ActorTemplate") +} + func TestRevisionDigestExcludesProvenance(t *testing.T) { revision := &Revision{Namespace: "agents", AgentName: "helper", Provenance: []byte(`[{"kind":"ConfigMap","hash":"first"}]`)} first, err := revision.Digest() diff --git a/go/core/internal/translator/skills_test.go b/go/core/internal/translator/skills_test.go index 65d8519d80..3ced68d71d 100644 --- a/go/core/internal/translator/skills_test.go +++ b/go/core/internal/translator/skills_test.go @@ -21,33 +21,11 @@ import ( func TestCompileSkillImageVolumes(t *testing.T) { const image = "registry.example.com/skills@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" - for _, kind := range []translator.HarnessType{translator.HarnessTypeKagent, translator.HarnessTypeCodex, translator.HarnessTypeClaude, translator.HarnessTypeBYO} { + for _, kind := range skillHarnessTypes { t.Run(string(kind), func(t *testing.T) { - harness := &v1alpha3.Harness{ObjectMeta: metav1.ObjectMeta{Name: "runtime", Namespace: "test"}, Spec: v1alpha3.HarnessSpec{ - Workload: v1alpha3.HarnessWorkload{Image: new(image), Command: []string{"/agent"}}, - Substrate: v1alpha3.RuntimeSubstratePolicy{WorkerPoolRef: corev1.LocalObjectReference{Name: "default"}, SnapshotPolicy: v1alpha3.RuntimeSnapshotPolicy{Location: "snapshots"}}, - }} - model := modelConfig() - model.Spec.APIKeySecret, model.Spec.APIKeySecretKey = "model-auth", "api-key" - switch kind { - case translator.HarnessTypeKagent: - harness.Spec.Kagent = &v1alpha3.KagentHarness{} - case translator.HarnessTypeCodex: - harness.Spec.Codex = &v1alpha3.CodexHarness{} - model.Spec.OpenAI = &v1alpha3.OpenAIConfig{APIFormat: new(v1alpha3.OpenAIAPIFormatResponses)} - case translator.HarnessTypeClaude: - harness.Spec.Claude = &v1alpha3.ClaudeHarness{} - model.Spec.Provider = v1alpha3.ModelProviderAnthropic - case translator.HarnessTypeBYO: - harness.Spec.BYO = &v1alpha3.BYOHarness{} - } - secret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "model-auth", Namespace: "test"}, Data: map[string][]byte{"api-key": []byte("test")}} - template := &v1alpha3.AgentTemplate{ObjectMeta: metav1.ObjectMeta{Name: "skills", Namespace: "test"}, Spec: v1alpha3.AgentTemplateSpec{ - ModelConfig: &v1alpha3.ObjectReference{Name: model.Name}, SystemPrompt: "help", - Skills: []v1alpha3.AgentTemplateSkill{{Name: "review", Source: v1alpha3.ArtifactSource{OCI: image, Path: "skills/review"}}}, - Plugins: []v1alpha3.PluginBundle{{Source: v1alpha3.ArtifactSource{OCI: image, Path: "plugin"}, Skills: []string{"deploy"}}}, - }} - c := compiler(t, harness, model, secret) + c, harness, template := skillCompiler(t, kind, image) + template.Spec.Skills = []v1alpha3.AgentTemplateSkill{{Name: "review", Source: v1alpha3.ArtifactSource{OCI: image, Path: "skills/review"}}} + template.Spec.Plugins = []v1alpha3.PluginBundle{{Source: v1alpha3.ArtifactSource{OCI: image, Path: "plugin"}, Skills: []string{"deploy"}}} result, err := c.CompileAgent(t.Context(), inlineAgent(harness, template)) require.NoError(t, err) require.Equal(t, []translator.SkillImage{image}, result.SkillImages) @@ -98,6 +76,69 @@ func TestCompileSkillImageVolumes(t *testing.T) { } } +var skillHarnessTypes = []translator.HarnessType{translator.HarnessTypeKagent, translator.HarnessTypeCodex, translator.HarnessTypeClaude, translator.HarnessTypeBYO} + +// skillCompiler returns a compiler for one Harness type and an AgentTemplate +// with no skills that the caller fills in. +func skillCompiler(t *testing.T, kind translator.HarnessType, image string) (*translator.Compiler, *v1alpha3.Harness, *v1alpha3.AgentTemplate) { + t.Helper() + harness := &v1alpha3.Harness{ObjectMeta: metav1.ObjectMeta{Name: "runtime", Namespace: "test"}, Spec: v1alpha3.HarnessSpec{ + Workload: v1alpha3.HarnessWorkload{Image: new(image), Command: []string{"/agent"}}, + Substrate: v1alpha3.RuntimeSubstratePolicy{WorkerPoolRef: corev1.LocalObjectReference{Name: "default"}, SnapshotPolicy: v1alpha3.RuntimeSnapshotPolicy{Location: "snapshots"}}, + }} + model := modelConfig() + model.Spec.APIKeySecret, model.Spec.APIKeySecretKey = "model-auth", "api-key" + switch kind { + case translator.HarnessTypeKagent: + harness.Spec.Kagent = &v1alpha3.KagentHarness{} + case translator.HarnessTypeCodex: + harness.Spec.Codex = &v1alpha3.CodexHarness{} + model.Spec.OpenAI = &v1alpha3.OpenAIConfig{APIFormat: new(v1alpha3.OpenAIAPIFormatResponses)} + case translator.HarnessTypeClaude: + harness.Spec.Claude = &v1alpha3.ClaudeHarness{} + model.Spec.Provider = v1alpha3.ModelProviderAnthropic + case translator.HarnessTypeBYO: + harness.Spec.BYO = &v1alpha3.BYOHarness{} + } + secret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "model-auth", Namespace: "test"}, Data: map[string][]byte{"api-key": []byte("test")}} + template := &v1alpha3.AgentTemplate{ObjectMeta: metav1.ObjectMeta{Name: "skills", Namespace: "test"}, Spec: v1alpha3.AgentTemplateSpec{ + ModelConfig: &v1alpha3.ObjectReference{Name: model.Name}, SystemPrompt: "help", + }} + return compiler(t, harness, model, secret), harness, template +} + +// The golden actor fetches git and S3 sources before its snapshot, so each +// Harness must report their origins for the golden actor's egress policy. +func TestCompileGoldenEgressDestinations(t *testing.T) { + const image = "registry.example.com/skills@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + want := []string{"http://git.internal.example:8080", "https://git.example.com:443", "https://objects.example.com:443"} + for _, kind := range skillHarnessTypes { + t.Run(string(kind), func(t *testing.T) { + c, harness, template := skillCompiler(t, kind, image) + template.Spec.Skills = []v1alpha3.AgentTemplateSkill{ + {Name: "review", Source: v1alpha3.ArtifactSource{Git: &v1alpha3.GitArtifact{URL: "https://git.example.com/skills.git", Commit: strings.Repeat("a", 40)}}}, + {Name: "triage", Source: v1alpha3.ArtifactSource{Git: &v1alpha3.GitArtifact{URL: "https://git.example.com/other.git", Commit: strings.Repeat("b", 40)}}}, + {Name: "summary", Source: v1alpha3.ArtifactSource{OCI: image}}, + } + template.Spec.Plugins = []v1alpha3.PluginBundle{ + {Source: v1alpha3.ArtifactSource{Git: &v1alpha3.GitArtifact{URL: "http://git.internal.example:8080/plugin.git", Commit: strings.Repeat("c", 40)}}}, + {Source: v1alpha3.ArtifactSource{Bucket: &v1alpha3.BucketArtifact{S3: v1alpha3.S3Object{ + Endpoint: "https://objects.example.com", Bucket: "plugins", Key: "plugin.zip", VersionID: "version-1", + }}}}, + } + result, err := c.CompileAgent(t.Context(), inlineAgent(harness, template)) + require.NoError(t, err) + require.Equal(t, want, result.GoldenEgressDestinations) + require.Subset(t, result.EgressDestinations, want, "sessions keep access to skill sources") + + template.Spec.Skills, template.Spec.Plugins = template.Spec.Skills[2:], nil + result, err = c.CompileAgent(t.Context(), inlineAgent(harness, template)) + require.NoError(t, err) + require.Empty(t, result.GoldenEgressDestinations, "OCI sources are mounted, not fetched") + }) + } +} + func TestCompileSkillSourceNormalization(t *testing.T) { const digest = "@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" var directory string diff --git a/go/core/pkg/install/plan.go b/go/core/pkg/install/plan.go index ca46cdb428..5ad54ac165 100644 --- a/go/core/pkg/install/plan.go +++ b/go/core/pkg/install/plan.go @@ -197,6 +197,7 @@ func postgresClientValues(namespace, serviceAccount string, includeBundledPostgr func substrateInstallValues(namespace, serviceAccount string) string { return fmt.Sprintf(`preview: - PerTemplateSnapshotLocation + - GoldenEgressPolicy ateApi: authorization: bootstrapOwners: diff --git a/go/core/pkg/install/values_test.go b/go/core/pkg/install/values_test.go index 763a259c79..837e5f2152 100644 --- a/go/core/pkg/install/values_test.go +++ b/go/core/pkg/install/values_test.go @@ -25,7 +25,7 @@ func TestSubstrateInstallValues(t *testing.T) { } `json:"credentialProvider"` } require.NoError(t, yaml.Unmarshal([]byte(substrateInstallValues(namespace, "kagent-controller")), &values)) - require.Contains(t, values.Preview, "PerTemplateSnapshotLocation") + require.Subset(t, values.Preview, []string{"PerTemplateSnapshotLocation", "GoldenEgressPolicy"}) require.Equal(t, []string{ "spiffe://cluster.local/ns/" + namespace + "/sa/kagent-controller", "system:serviceaccount:ate-system:ate-client", diff --git a/helm/README.md b/helm/README.md index e359d40af0..d69707f3d0 100644 --- a/helm/README.md +++ b/helm/README.md @@ -55,10 +55,14 @@ recreated Actors, Tags, and WorkerPools; Substrate does not migrate the older snapshot metadata. WorkerPools use `spec.sandboxClasses[]`, with an existing `configRef` of the same runtime class. -Kagent configures snapshot storage per Harness or SandboxTemplate. Substrate -v0.7.0-alpha1 requires `preview: [PerTemplateSnapshotLocation]` in its Helm values to -honor those locations. `kagent install` enables this gate by default; retain it -when supplying your own `preview` list or installing Substrate directly. +Substrate v0.7.0-alpha1 needs two preview gates in its Helm values: +`preview: [PerTemplateSnapshotLocation, GoldenEgressPolicy]`. +`PerTemplateSnapshotLocation` lets each Harness or SandboxTemplate set its own +snapshot storage. `GoldenEgressPolicy` lets the golden actor fetch git and S3 +skill sources. `kagent install` turns on both gates. Keep both when you set your +own `preview` list or install Substrate yourself. Without `GoldenEgressPolicy`, +Agents with git or S3 skill sources report `ActorTemplateRejected` until you +turn on the gate. Runtime-facing URLs must use full DNS names because Substrate only permits DNS queries matching the egress policy. The chart generates controller and API tool