diff --git a/go/api/config/crd/bases/kagent.dev_sandboxagents.yaml b/go/api/config/crd/bases/kagent.dev_sandboxagents.yaml index 8dd8560b8..e0cf48ec8 100644 --- a/go/api/config/crd/bases/kagent.dev_sandboxagents.yaml +++ b/go/api/config/crd/bases/kagent.dev_sandboxagents.yaml @@ -11369,6 +11369,17 @@ spec: x-kubernetes-validations: - message: spec.skills is not supported for sandbox agents rule: '!has(self.skills)' + - message: 'deployment.nodeSelector is not supported for sandbox agents: + substrate schedules actors onto WorkerPool workers, so set the WorkerPool''s + nodeSelector instead' + rule: '!has(self.declarative) || !has(self.declarative.deployment) || + !has(self.declarative.deployment.nodeSelector) || size(self.declarative.deployment.nodeSelector) + == 0' + - message: 'deployment.nodeSelector is not supported for sandbox agents: + substrate schedules actors onto WorkerPool workers, so set the WorkerPool''s + nodeSelector instead' + rule: '!has(self.byo) || !has(self.byo.deployment) || !has(self.byo.deployment.nodeSelector) + || size(self.byo.deployment.nodeSelector) == 0' - message: type must be specified rule: has(self.type) - message: type must be either Declarative or BYO diff --git a/go/api/v1alpha2/agent_spec_validation.go b/go/api/v1alpha2/agent_spec_validation.go index 5e46592fb..a7c6b328a 100644 --- a/go/api/v1alpha2/agent_spec_validation.go +++ b/go/api/v1alpha2/agent_spec_validation.go @@ -6,8 +6,9 @@ import ( ) const ( - substrateSandboxSkillsUnsupportedMsg = "spec.skills is not supported for sandbox agents" - substrateSandboxBYOMissingCommandMsg = "BYO agents on substrate must set spec.byo.deployment.cmd (substrate does not fall back to the image entrypoint)" + substrateSandboxSkillsUnsupportedMsg = "spec.skills is not supported for sandbox agents" + substrateSandboxBYOMissingCommandMsg = "BYO agents on substrate must set spec.byo.deployment.cmd (substrate does not fall back to the image entrypoint)" + substrateSandboxNodeSelectorUnsupportedMsg = "deployment.nodeSelector is not supported for sandbox agents: substrate schedules actors onto WorkerPool workers, so set the WorkerPool's nodeSelector instead" ) // AgentSpecHasSkills reports whether the spec configures any skill sources. @@ -23,7 +24,11 @@ func AgentSpecHasSkills(spec *AgentSpec) bool { // does not support on Agent Substrate (for example declarative skills). Declarative // Python/Go and BYO (Go/Python) agents are supported; BYO agents must provide an explicit // command because substrate copies the container Command verbatim with no image-entrypoint -// fallback. +// fallback. A per-agent deployment.nodeSelector is rejected: substrate ActorTemplates carry +// no node placement (actors run on WorkerPool workers), so the selector would otherwise be +// silently dropped. The skills and nodeSelector checks are also enforced at admission by CEL +// rules on SandboxAgentSpec; this function keeps them effective for objects created before +// those rules shipped and for callers that bypass the API server. func ValidateSubstrateSandboxAgentSpec(agent *SandboxAgent) error { if agent == nil { return nil @@ -32,6 +37,9 @@ func ValidateSubstrateSandboxAgentSpec(agent *SandboxAgent) error { if AgentSpecHasSkills(spec) { return fmt.Errorf("%s", substrateSandboxSkillsUnsupportedMsg) } + if len(agentSpecNodeSelector(spec)) > 0 { + return fmt.Errorf("%s", substrateSandboxNodeSelectorUnsupportedMsg) + } if spec.Type == AgentType_BYO { dep := spec.BYO // Trim so a whitespace-only cmd is rejected like an empty one (substrate would treat it @@ -42,3 +50,18 @@ func ValidateSubstrateSandboxAgentSpec(agent *SandboxAgent) error { } return nil } + +// agentSpecNodeSelector returns the per-agent deployment nodeSelector, whichever agent +// type carries it. +func agentSpecNodeSelector(spec *AgentSpec) map[string]string { + if spec == nil { + return nil + } + if spec.Declarative != nil && spec.Declarative.Deployment != nil { + return spec.Declarative.Deployment.NodeSelector + } + if spec.BYO != nil && spec.BYO.Deployment != nil { + return spec.BYO.Deployment.NodeSelector + } + return nil +} diff --git a/go/api/v1alpha2/agent_spec_validation_test.go b/go/api/v1alpha2/agent_spec_validation_test.go index 704edad3a..ea9b09b76 100644 --- a/go/api/v1alpha2/agent_spec_validation_test.go +++ b/go/api/v1alpha2/agent_spec_validation_test.go @@ -81,6 +81,67 @@ func TestValidateSubstrateSandboxAgentSpec(t *testing.T) { require.NoError(t, ValidateSubstrateSandboxAgentSpec(agent)) }) + t.Run("rejects declarative agents with a nodeSelector", func(t *testing.T) { + agent := &SandboxAgent{ + Spec: SandboxAgentSpec{ + AgentSpec: AgentSpec{ + Type: AgentType_Declarative, + Declarative: &DeclarativeAgentSpec{ + Runtime: DeclarativeRuntime_Go, + Deployment: &DeclarativeDeploymentSpec{ + SharedDeploymentSpec: SharedDeploymentSpec{ + NodeSelector: map[string]string{"kubernetes.io/arch": "amd64", "topology.kubernetes.io/zone": "z1"}, + }, + }, + }, + }, + }, + } + err := ValidateSubstrateSandboxAgentSpec(agent) + require.Error(t, err) + require.Contains(t, err.Error(), substrateSandboxNodeSelectorUnsupportedMsg) + }) + + t.Run("rejects BYO agents with a nodeSelector", func(t *testing.T) { + cmd := "/app" + agent := &SandboxAgent{ + Spec: SandboxAgentSpec{ + AgentSpec: AgentSpec{ + Type: AgentType_BYO, + BYO: &BYOAgentSpec{Deployment: &ByoDeploymentSpec{ + Image: "example/agent:latest", + Cmd: &cmd, + SharedDeploymentSpec: SharedDeploymentSpec{ + NodeSelector: map[string]string{"kubernetes.io/arch": "amd64"}, + }, + }}, + }, + }, + } + err := ValidateSubstrateSandboxAgentSpec(agent) + require.Error(t, err) + require.Contains(t, err.Error(), substrateSandboxNodeSelectorUnsupportedMsg) + }) + + t.Run("allows declarative agents with an empty nodeSelector", func(t *testing.T) { + agent := &SandboxAgent{ + Spec: SandboxAgentSpec{ + AgentSpec: AgentSpec{ + Type: AgentType_Declarative, + Declarative: &DeclarativeAgentSpec{ + Runtime: DeclarativeRuntime_Go, + Deployment: &DeclarativeDeploymentSpec{ + SharedDeploymentSpec: SharedDeploymentSpec{ + NodeSelector: map[string]string{}, + }, + }, + }, + }, + }, + } + require.NoError(t, ValidateSubstrateSandboxAgentSpec(agent)) + }) + t.Run("allows go runtime", func(t *testing.T) { agent := &SandboxAgent{ Spec: SandboxAgentSpec{ diff --git a/go/api/v1alpha2/sandboxagent_cel_test.go b/go/api/v1alpha2/sandboxagent_cel_test.go new file mode 100644 index 000000000..cd2316416 --- /dev/null +++ b/go/api/v1alpha2/sandboxagent_cel_test.go @@ -0,0 +1,155 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha2 + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + ctrl_client "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest" +) + +// TestSandboxAgentCELValidation pins the SandboxAgentSpec CEL rules against a +// real kube-apiserver loaded with the shipped CRDs, so admission rejects +// unsupported configuration instead of the controller discovering it at +// reconcile time. ValidateSubstrateSandboxAgentSpec mirrors these rules in Go +// for objects that predate them. +func TestSandboxAgentCELValidation(t *testing.T) { + testEnv := &envtest.Environment{ + BinaryAssetsDirectory: envtestAssetsDir(t), + CRDDirectoryPaths: []string{crdBasesDir(t)}, + ErrorIfCRDPathMissing: true, + } + cfg, err := testEnv.Start() + require.NoError(t, err) + t.Cleanup(func() { _ = testEnv.Stop() }) + + scheme := runtime.NewScheme() + require.NoError(t, corev1.AddToScheme(scheme)) + require.NoError(t, AddToScheme(scheme)) + cl, err := ctrl_client.New(cfg, ctrl_client.Options{Scheme: scheme}) + require.NoError(t, err) + + ctx := context.Background() + const ns = "sandbox-cel" + require.NoError(t, cl.Create(ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: ns}})) + + cmd := "/app" + cases := []struct { + name string + build func() ctrl_client.Object + wantReject string // substring in admission error; empty means accept + }{ + { + name: "declarative nodeSelector rejected", + build: func() ctrl_client.Object { + return &SandboxAgent{ + ObjectMeta: metav1.ObjectMeta{Name: "sa-decl-nodeselector", Namespace: ns}, + Spec: SandboxAgentSpec{ + AgentSpec: AgentSpec{ + Type: AgentType_Declarative, + Declarative: &DeclarativeAgentSpec{ + Runtime: DeclarativeRuntime_Go, + Deployment: &DeclarativeDeploymentSpec{ + SharedDeploymentSpec: SharedDeploymentSpec{ + NodeSelector: map[string]string{"kubernetes.io/arch": "amd64"}, + }, + }, + }, + }, + }, + } + }, + wantReject: "deployment.nodeSelector is not supported for sandbox agents", + }, + { + name: "byo nodeSelector rejected", + build: func() ctrl_client.Object { + return &SandboxAgent{ + ObjectMeta: metav1.ObjectMeta{Name: "sa-byo-nodeselector", Namespace: ns}, + Spec: SandboxAgentSpec{ + AgentSpec: AgentSpec{ + Type: AgentType_BYO, + BYO: &BYOAgentSpec{Deployment: &ByoDeploymentSpec{ + Image: "example/agent:latest", + Cmd: &cmd, + SharedDeploymentSpec: SharedDeploymentSpec{ + NodeSelector: map[string]string{"kubernetes.io/arch": "amd64"}, + }, + }}, + }, + }, + } + }, + wantReject: "deployment.nodeSelector is not supported for sandbox agents", + }, + { + name: "declarative empty nodeSelector accepted", + build: func() ctrl_client.Object { + return &SandboxAgent{ + ObjectMeta: metav1.ObjectMeta{Name: "sa-decl-empty-nodeselector", Namespace: ns}, + Spec: SandboxAgentSpec{ + AgentSpec: AgentSpec{ + Type: AgentType_Declarative, + Declarative: &DeclarativeAgentSpec{ + Runtime: DeclarativeRuntime_Go, + Deployment: &DeclarativeDeploymentSpec{ + SharedDeploymentSpec: SharedDeploymentSpec{ + NodeSelector: map[string]string{}, + }, + }, + }, + }, + }, + } + }, + }, + { + name: "declarative without nodeSelector accepted", + build: func() ctrl_client.Object { + return &SandboxAgent{ + ObjectMeta: metav1.ObjectMeta{Name: "sa-decl-no-nodeselector", Namespace: ns}, + Spec: SandboxAgentSpec{ + AgentSpec: AgentSpec{ + Type: AgentType_Declarative, + Declarative: &DeclarativeAgentSpec{ + Runtime: DeclarativeRuntime_Go, + }, + }, + }, + } + }, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + err := cl.Create(ctx, c.build()) + if c.wantReject == "" { + require.NoError(t, err) + return + } + require.Error(t, err) + require.Contains(t, err.Error(), c.wantReject) + }) + } +} diff --git a/go/api/v1alpha2/sandboxagent_types.go b/go/api/v1alpha2/sandboxagent_types.go index b1bceeeb1..fdefbf88b 100644 --- a/go/api/v1alpha2/sandboxagent_types.go +++ b/go/api/v1alpha2/sandboxagent_types.go @@ -38,6 +38,8 @@ type SandboxAgent struct { } // +kubebuilder:validation:XValidation:rule="!has(self.skills)",message="spec.skills is not supported for sandbox agents" +// +kubebuilder:validation:XValidation:rule="!has(self.declarative) || !has(self.declarative.deployment) || !has(self.declarative.deployment.nodeSelector) || size(self.declarative.deployment.nodeSelector) == 0",message="deployment.nodeSelector is not supported for sandbox agents: substrate schedules actors onto WorkerPool workers, so set the WorkerPool's nodeSelector instead" +// +kubebuilder:validation:XValidation:rule="!has(self.byo) || !has(self.byo.deployment) || !has(self.byo.deployment.nodeSelector) || size(self.byo.deployment.nodeSelector) == 0",message="deployment.nodeSelector is not supported for sandbox agents: substrate schedules actors onto WorkerPool workers, so set the WorkerPool's nodeSelector instead" type SandboxAgentSpec struct { AgentSpec `json:",inline"` diff --git a/helm/kagent-crds/templates/kagent.dev_sandboxagents.yaml b/helm/kagent-crds/templates/kagent.dev_sandboxagents.yaml index 8dd8560b8..e0cf48ec8 100644 --- a/helm/kagent-crds/templates/kagent.dev_sandboxagents.yaml +++ b/helm/kagent-crds/templates/kagent.dev_sandboxagents.yaml @@ -11369,6 +11369,17 @@ spec: x-kubernetes-validations: - message: spec.skills is not supported for sandbox agents rule: '!has(self.skills)' + - message: 'deployment.nodeSelector is not supported for sandbox agents: + substrate schedules actors onto WorkerPool workers, so set the WorkerPool''s + nodeSelector instead' + rule: '!has(self.declarative) || !has(self.declarative.deployment) || + !has(self.declarative.deployment.nodeSelector) || size(self.declarative.deployment.nodeSelector) + == 0' + - message: 'deployment.nodeSelector is not supported for sandbox agents: + substrate schedules actors onto WorkerPool workers, so set the WorkerPool''s + nodeSelector instead' + rule: '!has(self.byo) || !has(self.byo.deployment) || !has(self.byo.deployment.nodeSelector) + || size(self.byo.deployment.nodeSelector) == 0' - message: type must be specified rule: has(self.type) - message: type must be either Declarative or BYO