From 9e2f065ead822fe0241e453219635cd8e64b31e3 Mon Sep 17 00:00:00 2001 From: Sanchit2662 Date: Mon, 20 Apr 2026 23:45:50 +0530 Subject: [PATCH 1/7] feat: add NetworkPolicy support for sandbox isolation - Add SandboxNetworkPolicy API type (None/Restricted/Custom modes) - Build per-sandbox NetworkPolicy with router ingress always allowed - Support cross-namespace router via NamespaceSelector - Per-session NP override in CreateSandboxRequest (replace semantics) - Configurable router selector/namespace via chart values and CLI flags - Clean up NP on sandbox delete, rollback, and GC - Add networkpolicies RBAC to workloadmanager ClusterRole Signed-off-by: Sanchit2662 --- cmd/workload-manager/main.go | 28 + cmd/workload-manager/main_test.go | 78 +++ ...me.agentcube.volcano.sh_agentruntimes.yaml | 480 ++++++++++++++++++ ...agentcube.volcano.sh_codeinterpreters.yaml | 480 ++++++++++++++++++ .../base/templates/rbac/workloadmanager.yaml | 3 + .../base/templates/workloadmanager.yaml | 4 + manifests/charts/base/values.yaml | 9 + pkg/apis/runtime/v1alpha1/agent_type.go | 5 + .../runtime/v1alpha1/codeinterpreter_types.go | 5 + .../runtime/v1alpha1/networkpolicy_types.go | 106 ++++ .../runtime/v1alpha1/zz_generated.deepcopy.go | 154 ++++++ pkg/common/types/sandbox.go | 5 + pkg/workloadmanager/garbage_collection.go | 21 +- .../garbage_collection_test.go | 11 +- pkg/workloadmanager/handlers.go | 45 +- pkg/workloadmanager/handlers_test.go | 70 ++- pkg/workloadmanager/k8s_client.go | 21 +- pkg/workloadmanager/networkpolicy_builder.go | 149 ++++++ .../networkpolicy_builder_test.go | 271 ++++++++++ pkg/workloadmanager/server.go | 22 + pkg/workloadmanager/workload_builder.go | 40 +- 21 files changed, 1962 insertions(+), 45 deletions(-) create mode 100644 cmd/workload-manager/main_test.go create mode 100644 pkg/apis/runtime/v1alpha1/networkpolicy_types.go create mode 100644 pkg/workloadmanager/networkpolicy_builder.go create mode 100644 pkg/workloadmanager/networkpolicy_builder_test.go diff --git a/cmd/workload-manager/main.go b/cmd/workload-manager/main.go index 65f1d80c..352933fd 100644 --- a/cmd/workload-manager/main.go +++ b/cmd/workload-manager/main.go @@ -22,6 +22,7 @@ import ( "fmt" "os" "os/signal" + "strings" "syscall" "time" @@ -58,6 +59,13 @@ func main() { tlsCert = flag.String("tls-cert", "", "Path to TLS certificate file") tlsKey = flag.String("tls-key", "", "Path to TLS key file") enableAuth = flag.Bool("enable-auth", false, "Enable Authentication") + routerSelector = flag.String("router-selector", "app=agentcube-router", + "Pod label selector identifying the router pod, used for the mandatory "+ + "router→sandbox ingress rule when NetworkPolicy.Mode=Restricted. "+ + "Format: comma-separated key=value pairs (e.g. app=agentcube-router,tier=proxy)") + routerNamespace = flag.String("router-namespace", "", + "Namespace the router runs in. Used to build a cross-namespace NamespaceSelector "+ + "for the router→sandbox ingress rule. Defaults to the workloadmanager's own namespace.") ) // Initialize klog flags @@ -103,6 +111,8 @@ func main() { TLSCert: *tlsCert, TLSKey: *tlsKey, EnableAuth: *enableAuth, + RouterSelector: parseSelector(*routerSelector), + RouterNamespace: *routerNamespace, } // Create and initialize API server @@ -166,6 +176,24 @@ func main() { klog.Info("Server stopped") } +// parseSelector parses a comma-separated "key=value" label selector string into a map. +// Malformed pairs are silently skipped; callers should validate the resulting map is non-empty. +func parseSelector(s string) map[string]string { + m := make(map[string]string) + for _, pair := range strings.Split(s, ",") { + pair = strings.TrimSpace(pair) + if pair == "" { + continue + } + k, v, ok := strings.Cut(pair, "=") + if !ok || strings.TrimSpace(k) == "" { + continue + } + m[strings.TrimSpace(k)] = strings.TrimSpace(v) + } + return m +} + func setupControllers(mgr ctrl.Manager, sandboxReconciler *workloadmanager.SandboxReconciler, codeInterpreterReconciler *workloadmanager.CodeInterpreterReconciler) error { // Setup Sandbox controller if err := ctrl.NewControllerManagedBy(mgr). diff --git a/cmd/workload-manager/main_test.go b/cmd/workload-manager/main_test.go new file mode 100644 index 00000000..e07b6ba4 --- /dev/null +++ b/cmd/workload-manager/main_test.go @@ -0,0 +1,78 @@ +/* +Copyright The Volcano Authors. + +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 main + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestParseSelector(t *testing.T) { + tests := []struct { + name string + in string + want map[string]string + }{ + { + name: "empty string", + in: "", + want: map[string]string{}, + }, + { + name: "single pair", + in: "app=agentcube-router", + want: map[string]string{"app": "agentcube-router"}, + }, + { + name: "multiple pairs", + in: "app=agentcube-router,tier=proxy", + want: map[string]string{"app": "agentcube-router", "tier": "proxy"}, + }, + { + name: "whitespace around pairs and keys is trimmed", + in: " app = agentcube-router , tier = proxy ", + want: map[string]string{"app": "agentcube-router", "tier": "proxy"}, + }, + { + name: "malformed pairs (no '=') are dropped", + in: "app=agentcube-router,bogus,tier=proxy", + want: map[string]string{"app": "agentcube-router", "tier": "proxy"}, + }, + { + name: "empty key is dropped", + in: "=value,app=agentcube-router", + want: map[string]string{"app": "agentcube-router"}, + }, + { + name: "empty value is preserved (k8s allows it)", + in: "role=", + want: map[string]string{"role": ""}, + }, + { + name: "trailing comma ignored", + in: "app=agentcube-router,", + want: map[string]string{"app": "agentcube-router"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := parseSelector(tt.in) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/manifests/charts/base/crds/runtime.agentcube.volcano.sh_agentruntimes.yaml b/manifests/charts/base/crds/runtime.agentcube.volcano.sh_agentruntimes.yaml index a6508b34..460ec327 100644 --- a/manifests/charts/base/crds/runtime.agentcube.volcano.sh_agentruntimes.yaml +++ b/manifests/charts/base/crds/runtime.agentcube.volcano.sh_agentruntimes.yaml @@ -63,6 +63,486 @@ spec: type: string description: Labels to apply to the sandbox Pod. type: object + networkPolicy: + description: |- + NetworkPolicy defines the network isolation policy for sandbox pods created from + this template. Defaults to Mode=None (no policy created). + properties: + allowDNS: + description: |- + AllowDNS controls whether a DNS egress rule (port 53 UDP/TCP) is added when + Mode=Restricted. Defaults to true. + type: boolean + allowedEgress: + description: AllowedEgress lists additional egress allow rules + applied when Mode=Restricted. + items: + description: SandboxEgressRule describes an egress allow + rule for a sandbox pod. + properties: + cidrs: + description: CIDRs is the list of destination CIDR blocks. + items: + type: string + type: array + ports: + description: Ports restricts the rule to these destination + ports. An empty list matches all ports. + items: + description: SandboxNetworkPolicyPort describes a + port for a sandbox network policy rule. + properties: + port: + anyOf: + - type: integer + - type: string + description: Port is the destination port number + or named port. + x-kubernetes-int-or-string: true + protocol: + description: Protocol is TCP or UDP. Defaults + to TCP. + type: string + required: + - port + type: object + type: array + type: object + type: array + allowedIngress: + description: |- + AllowedIngress lists additional ingress allow rules when Mode=Restricted. + Router ingress is always permitted in Restricted mode regardless of this field. + items: + description: SandboxIngressRule describes an ingress allow + rule for a sandbox pod. + properties: + cidrs: + description: CIDRs is the list of source CIDR blocks. + items: + type: string + type: array + ports: + description: Ports restricts the rule to these destination + ports. An empty list matches all ports. + items: + description: SandboxNetworkPolicyPort describes a + port for a sandbox network policy rule. + properties: + port: + anyOf: + - type: integer + - type: string + description: Port is the destination port number + or named port. + x-kubernetes-int-or-string: true + protocol: + description: Protocol is TCP or UDP. Defaults + to TCP. + type: string + required: + - port + type: object + type: array + type: object + type: array + custom: + description: Custom provides raw NetworkPolicy ingress/egress + rules, used only when Mode=Custom. + properties: + egress: + description: Egress is the list of egress rules. + items: + description: |- + NetworkPolicyEgressRule describes a particular set of traffic that is allowed out of pods + matched by a NetworkPolicySpec's podSelector. The traffic must match both ports and to. + This type is beta-level in 1.8 + properties: + ports: + description: |- + ports is a list of destination ports for outgoing traffic. + Each item in this list is combined using a logical OR. If this field is + empty or missing, this rule matches all ports (traffic not restricted by port). + If this field is present and contains at least one item, then this rule allows + traffic only if the traffic matches at least one port in the list. + items: + description: NetworkPolicyPort describes a port + to allow traffic on + properties: + endPort: + description: |- + endPort indicates that the range of ports from port to endPort if set, inclusive, + should be allowed by the policy. This field cannot be defined if the port field + is not defined or if the port field is defined as a named (string) port. + The endPort must be equal or greater than port. + format: int32 + type: integer + port: + anyOf: + - type: integer + - type: string + description: |- + port represents the port on the given protocol. This can either be a numerical or named + port on a pod. If this field is not provided, this matches all port names and + numbers. + If present, only traffic on the specified protocol AND port will be matched. + x-kubernetes-int-or-string: true + protocol: + description: |- + protocol represents the protocol (TCP, UDP, or SCTP) which traffic must match. + If not specified, this field defaults to TCP. + type: string + type: object + type: array + x-kubernetes-list-type: atomic + to: + description: |- + to is a list of destinations for outgoing traffic of pods selected for this rule. + Items in this list are combined using a logical OR operation. If this field is + empty or missing, this rule matches all destinations (traffic not restricted by + destination). If this field is present and contains at least one item, this rule + allows traffic only if the traffic matches at least one item in the to list. + items: + description: |- + NetworkPolicyPeer describes a peer to allow traffic to/from. Only certain combinations of + fields are allowed + properties: + ipBlock: + description: |- + ipBlock defines policy on a particular IPBlock. If this field is set then + neither of the other fields can be. + properties: + cidr: + description: |- + cidr is a string representing the IPBlock + Valid examples are "192.168.1.0/24" or "2001:db8::/64" + type: string + except: + description: |- + except is a slice of CIDRs that should not be included within an IPBlock + Valid examples are "192.168.1.0/24" or "2001:db8::/64" + Except values will be rejected if they are outside the cidr range + items: + type: string + type: array + x-kubernetes-list-type: atomic + required: + - cidr + type: object + namespaceSelector: + description: |- + namespaceSelector selects namespaces using cluster-scoped labels. This field follows + standard label selector semantics; if present but empty, it selects all namespaces. + + If podSelector is also set, then the NetworkPolicyPeer as a whole selects + the pods matching podSelector in the namespaces selected by namespaceSelector. + Otherwise it selects all pods in the namespaces selected by namespaceSelector. + properties: + matchExpressions: + description: matchExpressions is a list + of label selector requirements. The + requirements are ANDed. + items: + description: |- + A label selector requirement is a selector that contains values, a key, and an operator that + relates the key and values. + properties: + key: + description: key is the label key + that the selector applies to. + type: string + operator: + description: |- + operator represents a key's relationship to a set of values. + Valid operators are In, NotIn, Exists and DoesNotExist. + type: string + values: + description: |- + values is an array of string values. If the operator is In or NotIn, + the values array must be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. This array is replaced during a strategic + merge patch. + items: + type: string + type: array + x-kubernetes-list-type: atomic + required: + - key + - operator + type: object + type: array + x-kubernetes-list-type: atomic + matchLabels: + additionalProperties: + type: string + description: |- + matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels + map is equivalent to an element of matchExpressions, whose key field is "key", the + operator is "In", and the values array contains only "value". The requirements are ANDed. + type: object + type: object + x-kubernetes-map-type: atomic + podSelector: + description: |- + podSelector is a label selector which selects pods. This field follows standard label + selector semantics; if present but empty, it selects all pods. + + If namespaceSelector is also set, then the NetworkPolicyPeer as a whole selects + the pods matching podSelector in the Namespaces selected by NamespaceSelector. + Otherwise it selects the pods matching podSelector in the policy's own namespace. + properties: + matchExpressions: + description: matchExpressions is a list + of label selector requirements. The + requirements are ANDed. + items: + description: |- + A label selector requirement is a selector that contains values, a key, and an operator that + relates the key and values. + properties: + key: + description: key is the label key + that the selector applies to. + type: string + operator: + description: |- + operator represents a key's relationship to a set of values. + Valid operators are In, NotIn, Exists and DoesNotExist. + type: string + values: + description: |- + values is an array of string values. If the operator is In or NotIn, + the values array must be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. This array is replaced during a strategic + merge patch. + items: + type: string + type: array + x-kubernetes-list-type: atomic + required: + - key + - operator + type: object + type: array + x-kubernetes-list-type: atomic + matchLabels: + additionalProperties: + type: string + description: |- + matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels + map is equivalent to an element of matchExpressions, whose key field is "key", the + operator is "In", and the values array contains only "value". The requirements are ANDed. + type: object + type: object + x-kubernetes-map-type: atomic + type: object + type: array + x-kubernetes-list-type: atomic + type: object + type: array + ingress: + description: Ingress is the list of ingress rules. + items: + description: |- + NetworkPolicyIngressRule describes a particular set of traffic that is allowed to the pods + matched by a NetworkPolicySpec's podSelector. The traffic must match both ports and from. + properties: + from: + description: |- + from is a list of sources which should be able to access the pods selected for this rule. + Items in this list are combined using a logical OR operation. If this field is + empty or missing, this rule matches all sources (traffic not restricted by + source). If this field is present and contains at least one item, this rule + allows traffic only if the traffic matches at least one item in the from list. + items: + description: |- + NetworkPolicyPeer describes a peer to allow traffic to/from. Only certain combinations of + fields are allowed + properties: + ipBlock: + description: |- + ipBlock defines policy on a particular IPBlock. If this field is set then + neither of the other fields can be. + properties: + cidr: + description: |- + cidr is a string representing the IPBlock + Valid examples are "192.168.1.0/24" or "2001:db8::/64" + type: string + except: + description: |- + except is a slice of CIDRs that should not be included within an IPBlock + Valid examples are "192.168.1.0/24" or "2001:db8::/64" + Except values will be rejected if they are outside the cidr range + items: + type: string + type: array + x-kubernetes-list-type: atomic + required: + - cidr + type: object + namespaceSelector: + description: |- + namespaceSelector selects namespaces using cluster-scoped labels. This field follows + standard label selector semantics; if present but empty, it selects all namespaces. + + If podSelector is also set, then the NetworkPolicyPeer as a whole selects + the pods matching podSelector in the namespaces selected by namespaceSelector. + Otherwise it selects all pods in the namespaces selected by namespaceSelector. + properties: + matchExpressions: + description: matchExpressions is a list + of label selector requirements. The + requirements are ANDed. + items: + description: |- + A label selector requirement is a selector that contains values, a key, and an operator that + relates the key and values. + properties: + key: + description: key is the label key + that the selector applies to. + type: string + operator: + description: |- + operator represents a key's relationship to a set of values. + Valid operators are In, NotIn, Exists and DoesNotExist. + type: string + values: + description: |- + values is an array of string values. If the operator is In or NotIn, + the values array must be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. This array is replaced during a strategic + merge patch. + items: + type: string + type: array + x-kubernetes-list-type: atomic + required: + - key + - operator + type: object + type: array + x-kubernetes-list-type: atomic + matchLabels: + additionalProperties: + type: string + description: |- + matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels + map is equivalent to an element of matchExpressions, whose key field is "key", the + operator is "In", and the values array contains only "value". The requirements are ANDed. + type: object + type: object + x-kubernetes-map-type: atomic + podSelector: + description: |- + podSelector is a label selector which selects pods. This field follows standard label + selector semantics; if present but empty, it selects all pods. + + If namespaceSelector is also set, then the NetworkPolicyPeer as a whole selects + the pods matching podSelector in the Namespaces selected by NamespaceSelector. + Otherwise it selects the pods matching podSelector in the policy's own namespace. + properties: + matchExpressions: + description: matchExpressions is a list + of label selector requirements. The + requirements are ANDed. + items: + description: |- + A label selector requirement is a selector that contains values, a key, and an operator that + relates the key and values. + properties: + key: + description: key is the label key + that the selector applies to. + type: string + operator: + description: |- + operator represents a key's relationship to a set of values. + Valid operators are In, NotIn, Exists and DoesNotExist. + type: string + values: + description: |- + values is an array of string values. If the operator is In or NotIn, + the values array must be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. This array is replaced during a strategic + merge patch. + items: + type: string + type: array + x-kubernetes-list-type: atomic + required: + - key + - operator + type: object + type: array + x-kubernetes-list-type: atomic + matchLabels: + additionalProperties: + type: string + description: |- + matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels + map is equivalent to an element of matchExpressions, whose key field is "key", the + operator is "In", and the values array contains only "value". The requirements are ANDed. + type: object + type: object + x-kubernetes-map-type: atomic + type: object + type: array + x-kubernetes-list-type: atomic + ports: + description: |- + ports is a list of ports which should be made accessible on the pods selected for + this rule. Each item in this list is combined using a logical OR. If this field is + empty or missing, this rule matches all ports (traffic not restricted by port). + If this field is present and contains at least one item, then this rule allows + traffic only if the traffic matches at least one port in the list. + items: + description: NetworkPolicyPort describes a port + to allow traffic on + properties: + endPort: + description: |- + endPort indicates that the range of ports from port to endPort if set, inclusive, + should be allowed by the policy. This field cannot be defined if the port field + is not defined or if the port field is defined as a named (string) port. + The endPort must be equal or greater than port. + format: int32 + type: integer + port: + anyOf: + - type: integer + - type: string + description: |- + port represents the port on the given protocol. This can either be a numerical or named + port on a pod. If this field is not provided, this matches all port names and + numbers. + If present, only traffic on the specified protocol AND port will be matched. + x-kubernetes-int-or-string: true + protocol: + description: |- + protocol represents the protocol (TCP, UDP, or SCTP) which traffic must match. + If not specified, this field defaults to TCP. + type: string + type: object + type: array + x-kubernetes-list-type: atomic + type: object + type: array + type: object + mode: + default: None + description: |- + Mode controls the network policy enforcement level. + "None" (default): no NetworkPolicy is created. + "Restricted": deny-all baseline; DNS egress and router ingress are always allowed. + "Custom": use the raw Ingress/Egress rules in the Custom field. + enum: + - None + - Restricted + - Custom + type: string + type: object spec: description: Spec is the Pod's spec properties: diff --git a/manifests/charts/base/crds/runtime.agentcube.volcano.sh_codeinterpreters.yaml b/manifests/charts/base/crds/runtime.agentcube.volcano.sh_codeinterpreters.yaml index a8cb4e3a..8535dd86 100644 --- a/manifests/charts/base/crds/runtime.agentcube.volcano.sh_codeinterpreters.yaml +++ b/manifests/charts/base/crds/runtime.agentcube.volcano.sh_codeinterpreters.yaml @@ -332,6 +332,486 @@ spec: type: string description: Labels to apply to the sandbox Pod. type: object + networkPolicy: + description: |- + NetworkPolicy defines the network isolation policy for sandbox pods created from + this template. Defaults to Mode=None (no policy created). + properties: + allowDNS: + description: |- + AllowDNS controls whether a DNS egress rule (port 53 UDP/TCP) is added when + Mode=Restricted. Defaults to true. + type: boolean + allowedEgress: + description: AllowedEgress lists additional egress allow rules + applied when Mode=Restricted. + items: + description: SandboxEgressRule describes an egress allow + rule for a sandbox pod. + properties: + cidrs: + description: CIDRs is the list of destination CIDR blocks. + items: + type: string + type: array + ports: + description: Ports restricts the rule to these destination + ports. An empty list matches all ports. + items: + description: SandboxNetworkPolicyPort describes a + port for a sandbox network policy rule. + properties: + port: + anyOf: + - type: integer + - type: string + description: Port is the destination port number + or named port. + x-kubernetes-int-or-string: true + protocol: + description: Protocol is TCP or UDP. Defaults + to TCP. + type: string + required: + - port + type: object + type: array + type: object + type: array + allowedIngress: + description: |- + AllowedIngress lists additional ingress allow rules when Mode=Restricted. + Router ingress is always permitted in Restricted mode regardless of this field. + items: + description: SandboxIngressRule describes an ingress allow + rule for a sandbox pod. + properties: + cidrs: + description: CIDRs is the list of source CIDR blocks. + items: + type: string + type: array + ports: + description: Ports restricts the rule to these destination + ports. An empty list matches all ports. + items: + description: SandboxNetworkPolicyPort describes a + port for a sandbox network policy rule. + properties: + port: + anyOf: + - type: integer + - type: string + description: Port is the destination port number + or named port. + x-kubernetes-int-or-string: true + protocol: + description: Protocol is TCP or UDP. Defaults + to TCP. + type: string + required: + - port + type: object + type: array + type: object + type: array + custom: + description: Custom provides raw NetworkPolicy ingress/egress + rules, used only when Mode=Custom. + properties: + egress: + description: Egress is the list of egress rules. + items: + description: |- + NetworkPolicyEgressRule describes a particular set of traffic that is allowed out of pods + matched by a NetworkPolicySpec's podSelector. The traffic must match both ports and to. + This type is beta-level in 1.8 + properties: + ports: + description: |- + ports is a list of destination ports for outgoing traffic. + Each item in this list is combined using a logical OR. If this field is + empty or missing, this rule matches all ports (traffic not restricted by port). + If this field is present and contains at least one item, then this rule allows + traffic only if the traffic matches at least one port in the list. + items: + description: NetworkPolicyPort describes a port + to allow traffic on + properties: + endPort: + description: |- + endPort indicates that the range of ports from port to endPort if set, inclusive, + should be allowed by the policy. This field cannot be defined if the port field + is not defined or if the port field is defined as a named (string) port. + The endPort must be equal or greater than port. + format: int32 + type: integer + port: + anyOf: + - type: integer + - type: string + description: |- + port represents the port on the given protocol. This can either be a numerical or named + port on a pod. If this field is not provided, this matches all port names and + numbers. + If present, only traffic on the specified protocol AND port will be matched. + x-kubernetes-int-or-string: true + protocol: + description: |- + protocol represents the protocol (TCP, UDP, or SCTP) which traffic must match. + If not specified, this field defaults to TCP. + type: string + type: object + type: array + x-kubernetes-list-type: atomic + to: + description: |- + to is a list of destinations for outgoing traffic of pods selected for this rule. + Items in this list are combined using a logical OR operation. If this field is + empty or missing, this rule matches all destinations (traffic not restricted by + destination). If this field is present and contains at least one item, this rule + allows traffic only if the traffic matches at least one item in the to list. + items: + description: |- + NetworkPolicyPeer describes a peer to allow traffic to/from. Only certain combinations of + fields are allowed + properties: + ipBlock: + description: |- + ipBlock defines policy on a particular IPBlock. If this field is set then + neither of the other fields can be. + properties: + cidr: + description: |- + cidr is a string representing the IPBlock + Valid examples are "192.168.1.0/24" or "2001:db8::/64" + type: string + except: + description: |- + except is a slice of CIDRs that should not be included within an IPBlock + Valid examples are "192.168.1.0/24" or "2001:db8::/64" + Except values will be rejected if they are outside the cidr range + items: + type: string + type: array + x-kubernetes-list-type: atomic + required: + - cidr + type: object + namespaceSelector: + description: |- + namespaceSelector selects namespaces using cluster-scoped labels. This field follows + standard label selector semantics; if present but empty, it selects all namespaces. + + If podSelector is also set, then the NetworkPolicyPeer as a whole selects + the pods matching podSelector in the namespaces selected by namespaceSelector. + Otherwise it selects all pods in the namespaces selected by namespaceSelector. + properties: + matchExpressions: + description: matchExpressions is a list + of label selector requirements. The + requirements are ANDed. + items: + description: |- + A label selector requirement is a selector that contains values, a key, and an operator that + relates the key and values. + properties: + key: + description: key is the label key + that the selector applies to. + type: string + operator: + description: |- + operator represents a key's relationship to a set of values. + Valid operators are In, NotIn, Exists and DoesNotExist. + type: string + values: + description: |- + values is an array of string values. If the operator is In or NotIn, + the values array must be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. This array is replaced during a strategic + merge patch. + items: + type: string + type: array + x-kubernetes-list-type: atomic + required: + - key + - operator + type: object + type: array + x-kubernetes-list-type: atomic + matchLabels: + additionalProperties: + type: string + description: |- + matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels + map is equivalent to an element of matchExpressions, whose key field is "key", the + operator is "In", and the values array contains only "value". The requirements are ANDed. + type: object + type: object + x-kubernetes-map-type: atomic + podSelector: + description: |- + podSelector is a label selector which selects pods. This field follows standard label + selector semantics; if present but empty, it selects all pods. + + If namespaceSelector is also set, then the NetworkPolicyPeer as a whole selects + the pods matching podSelector in the Namespaces selected by NamespaceSelector. + Otherwise it selects the pods matching podSelector in the policy's own namespace. + properties: + matchExpressions: + description: matchExpressions is a list + of label selector requirements. The + requirements are ANDed. + items: + description: |- + A label selector requirement is a selector that contains values, a key, and an operator that + relates the key and values. + properties: + key: + description: key is the label key + that the selector applies to. + type: string + operator: + description: |- + operator represents a key's relationship to a set of values. + Valid operators are In, NotIn, Exists and DoesNotExist. + type: string + values: + description: |- + values is an array of string values. If the operator is In or NotIn, + the values array must be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. This array is replaced during a strategic + merge patch. + items: + type: string + type: array + x-kubernetes-list-type: atomic + required: + - key + - operator + type: object + type: array + x-kubernetes-list-type: atomic + matchLabels: + additionalProperties: + type: string + description: |- + matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels + map is equivalent to an element of matchExpressions, whose key field is "key", the + operator is "In", and the values array contains only "value". The requirements are ANDed. + type: object + type: object + x-kubernetes-map-type: atomic + type: object + type: array + x-kubernetes-list-type: atomic + type: object + type: array + ingress: + description: Ingress is the list of ingress rules. + items: + description: |- + NetworkPolicyIngressRule describes a particular set of traffic that is allowed to the pods + matched by a NetworkPolicySpec's podSelector. The traffic must match both ports and from. + properties: + from: + description: |- + from is a list of sources which should be able to access the pods selected for this rule. + Items in this list are combined using a logical OR operation. If this field is + empty or missing, this rule matches all sources (traffic not restricted by + source). If this field is present and contains at least one item, this rule + allows traffic only if the traffic matches at least one item in the from list. + items: + description: |- + NetworkPolicyPeer describes a peer to allow traffic to/from. Only certain combinations of + fields are allowed + properties: + ipBlock: + description: |- + ipBlock defines policy on a particular IPBlock. If this field is set then + neither of the other fields can be. + properties: + cidr: + description: |- + cidr is a string representing the IPBlock + Valid examples are "192.168.1.0/24" or "2001:db8::/64" + type: string + except: + description: |- + except is a slice of CIDRs that should not be included within an IPBlock + Valid examples are "192.168.1.0/24" or "2001:db8::/64" + Except values will be rejected if they are outside the cidr range + items: + type: string + type: array + x-kubernetes-list-type: atomic + required: + - cidr + type: object + namespaceSelector: + description: |- + namespaceSelector selects namespaces using cluster-scoped labels. This field follows + standard label selector semantics; if present but empty, it selects all namespaces. + + If podSelector is also set, then the NetworkPolicyPeer as a whole selects + the pods matching podSelector in the namespaces selected by namespaceSelector. + Otherwise it selects all pods in the namespaces selected by namespaceSelector. + properties: + matchExpressions: + description: matchExpressions is a list + of label selector requirements. The + requirements are ANDed. + items: + description: |- + A label selector requirement is a selector that contains values, a key, and an operator that + relates the key and values. + properties: + key: + description: key is the label key + that the selector applies to. + type: string + operator: + description: |- + operator represents a key's relationship to a set of values. + Valid operators are In, NotIn, Exists and DoesNotExist. + type: string + values: + description: |- + values is an array of string values. If the operator is In or NotIn, + the values array must be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. This array is replaced during a strategic + merge patch. + items: + type: string + type: array + x-kubernetes-list-type: atomic + required: + - key + - operator + type: object + type: array + x-kubernetes-list-type: atomic + matchLabels: + additionalProperties: + type: string + description: |- + matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels + map is equivalent to an element of matchExpressions, whose key field is "key", the + operator is "In", and the values array contains only "value". The requirements are ANDed. + type: object + type: object + x-kubernetes-map-type: atomic + podSelector: + description: |- + podSelector is a label selector which selects pods. This field follows standard label + selector semantics; if present but empty, it selects all pods. + + If namespaceSelector is also set, then the NetworkPolicyPeer as a whole selects + the pods matching podSelector in the Namespaces selected by NamespaceSelector. + Otherwise it selects the pods matching podSelector in the policy's own namespace. + properties: + matchExpressions: + description: matchExpressions is a list + of label selector requirements. The + requirements are ANDed. + items: + description: |- + A label selector requirement is a selector that contains values, a key, and an operator that + relates the key and values. + properties: + key: + description: key is the label key + that the selector applies to. + type: string + operator: + description: |- + operator represents a key's relationship to a set of values. + Valid operators are In, NotIn, Exists and DoesNotExist. + type: string + values: + description: |- + values is an array of string values. If the operator is In or NotIn, + the values array must be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. This array is replaced during a strategic + merge patch. + items: + type: string + type: array + x-kubernetes-list-type: atomic + required: + - key + - operator + type: object + type: array + x-kubernetes-list-type: atomic + matchLabels: + additionalProperties: + type: string + description: |- + matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels + map is equivalent to an element of matchExpressions, whose key field is "key", the + operator is "In", and the values array contains only "value". The requirements are ANDed. + type: object + type: object + x-kubernetes-map-type: atomic + type: object + type: array + x-kubernetes-list-type: atomic + ports: + description: |- + ports is a list of ports which should be made accessible on the pods selected for + this rule. Each item in this list is combined using a logical OR. If this field is + empty or missing, this rule matches all ports (traffic not restricted by port). + If this field is present and contains at least one item, then this rule allows + traffic only if the traffic matches at least one port in the list. + items: + description: NetworkPolicyPort describes a port + to allow traffic on + properties: + endPort: + description: |- + endPort indicates that the range of ports from port to endPort if set, inclusive, + should be allowed by the policy. This field cannot be defined if the port field + is not defined or if the port field is defined as a named (string) port. + The endPort must be equal or greater than port. + format: int32 + type: integer + port: + anyOf: + - type: integer + - type: string + description: |- + port represents the port on the given protocol. This can either be a numerical or named + port on a pod. If this field is not provided, this matches all port names and + numbers. + If present, only traffic on the specified protocol AND port will be matched. + x-kubernetes-int-or-string: true + protocol: + description: |- + protocol represents the protocol (TCP, UDP, or SCTP) which traffic must match. + If not specified, this field defaults to TCP. + type: string + type: object + type: array + x-kubernetes-list-type: atomic + type: object + type: array + type: object + mode: + default: None + description: |- + Mode controls the network policy enforcement level. + "None" (default): no NetworkPolicy is created. + "Restricted": deny-all baseline; DNS egress and router ingress are always allowed. + "Custom": use the raw Ingress/Egress rules in the Custom field. + enum: + - None + - Restricted + - Custom + type: string + type: object resources: description: |- Compute Resources required by this container. diff --git a/manifests/charts/base/templates/rbac/workloadmanager.yaml b/manifests/charts/base/templates/rbac/workloadmanager.yaml index 73f6e3b0..9db63116 100644 --- a/manifests/charts/base/templates/rbac/workloadmanager.yaml +++ b/manifests/charts/base/templates/rbac/workloadmanager.yaml @@ -49,6 +49,9 @@ rules: - apiGroups: [""] resources: ["secrets"] verbs: ["get", "list", "watch", "create", "update", "patch", "delete"] + - apiGroups: ["networking.k8s.io"] + resources: ["networkpolicies"] + verbs: ["get", "list", "watch", "create", "delete"] --- apiVersion: rbac.authorization.k8s.io/v1 diff --git a/manifests/charts/base/templates/workloadmanager.yaml b/manifests/charts/base/templates/workloadmanager.yaml index 04d84df0..4e32bed9 100644 --- a/manifests/charts/base/templates/workloadmanager.yaml +++ b/manifests/charts/base/templates/workloadmanager.yaml @@ -43,6 +43,10 @@ spec: args: - --port={{ .Values.workloadmanager.service.port }} - --runtime-class-name= + - --router-selector={{ .Values.workloadmanager.networkPolicy.routerSelector }} + {{- if .Values.workloadmanager.networkPolicy.routerNamespace }} + - --router-namespace={{ .Values.workloadmanager.networkPolicy.routerNamespace }} + {{- end }} resources: {{- toYaml .Values.workloadmanager.resources | nindent 12 }} livenessProbe: diff --git a/manifests/charts/base/values.yaml b/manifests/charts/base/values.yaml index 25a07979..806a54de 100644 --- a/manifests/charts/base/values.yaml +++ b/manifests/charts/base/values.yaml @@ -53,6 +53,15 @@ workloadmanager: cpu: 100m memory: 128Mi extraEnv: [] + networkPolicy: + # routerSelector is the pod label selector used to identify the router pod. + # Applied as the mandatory ingress allow rule when SandboxNetworkPolicy.Mode=Restricted. + # Format: comma-separated key=value pairs, e.g. "app=agentcube-router,tier=proxy" + routerSelector: "app=agentcube-router" + # routerNamespace is the namespace the router runs in. + # Used to build the cross-namespace NamespaceSelector for the router→sandbox ingress rule. + # Defaults to the workloadmanager's own namespace when left empty. + routerNamespace: "" # Volcano Agent Scheduler volcano: diff --git a/pkg/apis/runtime/v1alpha1/agent_type.go b/pkg/apis/runtime/v1alpha1/agent_type.go index fe5413fa..9de71890 100644 --- a/pkg/apis/runtime/v1alpha1/agent_type.go +++ b/pkg/apis/runtime/v1alpha1/agent_type.go @@ -78,6 +78,11 @@ type SandboxTemplate struct { // Spec is the Pod's spec // +kubebuilder:validation:Required Spec corev1.PodSpec `json:"spec" protobuf:"bytes,3,opt,name=spec"` + + // NetworkPolicy defines the network isolation policy for sandbox pods created from + // this template. Defaults to Mode=None (no policy created). + // +optional + NetworkPolicy *SandboxNetworkPolicy `json:"networkPolicy,omitempty"` } // AgentRuntimeList contains a list of AgentRuntime diff --git a/pkg/apis/runtime/v1alpha1/codeinterpreter_types.go b/pkg/apis/runtime/v1alpha1/codeinterpreter_types.go index c12154f3..90533705 100644 --- a/pkg/apis/runtime/v1alpha1/codeinterpreter_types.go +++ b/pkg/apis/runtime/v1alpha1/codeinterpreter_types.go @@ -151,6 +151,11 @@ type CodeInterpreterSandboxTemplate struct { // More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ // +optional Resources corev1.ResourceRequirements `json:"resources,omitempty"` + + // NetworkPolicy defines the network isolation policy for sandbox pods created from + // this template. Defaults to Mode=None (no policy created). + // +optional + NetworkPolicy *SandboxNetworkPolicy `json:"networkPolicy,omitempty"` } // TargetPort defines a port that the runtime will expose. diff --git a/pkg/apis/runtime/v1alpha1/networkpolicy_types.go b/pkg/apis/runtime/v1alpha1/networkpolicy_types.go new file mode 100644 index 00000000..ff5dcfb4 --- /dev/null +++ b/pkg/apis/runtime/v1alpha1/networkpolicy_types.go @@ -0,0 +1,106 @@ +/* +Copyright The Volcano Authors. + +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 v1alpha1 + +import ( + corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" + "k8s.io/apimachinery/pkg/util/intstr" +) + +// NetworkPolicyMode defines the network policy enforcement level for a sandbox. +// +kubebuilder:validation:Enum=None;Restricted;Custom +type NetworkPolicyMode string + +const ( + // NetworkPolicyModeNone creates no NetworkPolicy (default, backward-compatible). + NetworkPolicyModeNone NetworkPolicyMode = "None" + // NetworkPolicyModeRestricted applies a deny-all baseline with explicit allow rules + // for DNS egress and router ingress. + NetworkPolicyModeRestricted NetworkPolicyMode = "Restricted" + // NetworkPolicyModeCustom uses the raw rules from the Custom field. + NetworkPolicyModeCustom NetworkPolicyMode = "Custom" +) + +// SandboxNetworkPolicy defines the network isolation policy applied to each sandbox pod. +type SandboxNetworkPolicy struct { + // Mode controls the network policy enforcement level. + // "None" (default): no NetworkPolicy is created. + // "Restricted": deny-all baseline; DNS egress and router ingress are always allowed. + // "Custom": use the raw Ingress/Egress rules in the Custom field. + // +kubebuilder:default=None + // +optional + Mode NetworkPolicyMode `json:"mode,omitempty"` + + // AllowedEgress lists additional egress allow rules applied when Mode=Restricted. + // +optional + AllowedEgress []SandboxEgressRule `json:"allowedEgress,omitempty"` + + // AllowDNS controls whether a DNS egress rule (port 53 UDP/TCP) is added when + // Mode=Restricted. Defaults to true. + // +optional + AllowDNS *bool `json:"allowDNS,omitempty"` + + // AllowedIngress lists additional ingress allow rules when Mode=Restricted. + // Router ingress is always permitted in Restricted mode regardless of this field. + // +optional + AllowedIngress []SandboxIngressRule `json:"allowedIngress,omitempty"` + + // Custom provides raw NetworkPolicy ingress/egress rules, used only when Mode=Custom. + // +optional + Custom *SandboxNetworkPolicyCustomRules `json:"custom,omitempty"` +} + +// SandboxEgressRule describes an egress allow rule for a sandbox pod. +type SandboxEgressRule struct { + // CIDRs is the list of destination CIDR blocks. + // +optional + CIDRs []string `json:"cidrs,omitempty"` + // Ports restricts the rule to these destination ports. An empty list matches all ports. + // +optional + Ports []SandboxNetworkPolicyPort `json:"ports,omitempty"` +} + +// SandboxIngressRule describes an ingress allow rule for a sandbox pod. +type SandboxIngressRule struct { + // CIDRs is the list of source CIDR blocks. + // +optional + CIDRs []string `json:"cidrs,omitempty"` + // Ports restricts the rule to these destination ports. An empty list matches all ports. + // +optional + Ports []SandboxNetworkPolicyPort `json:"ports,omitempty"` +} + +// SandboxNetworkPolicyPort describes a port for a sandbox network policy rule. +type SandboxNetworkPolicyPort struct { + // Protocol is TCP or UDP. Defaults to TCP. + // +optional + Protocol *corev1.Protocol `json:"protocol,omitempty"` + // Port is the destination port number or named port. + Port intstr.IntOrString `json:"port"` +} + +// SandboxNetworkPolicyCustomRules holds raw Kubernetes NetworkPolicy ingress/egress rules. +// Used only when Mode=Custom. +type SandboxNetworkPolicyCustomRules struct { + // Ingress is the list of ingress rules. + // +optional + Ingress []networkingv1.NetworkPolicyIngressRule `json:"ingress,omitempty"` + // Egress is the list of egress rules. + // +optional + Egress []networkingv1.NetworkPolicyEgressRule `json:"egress,omitempty"` +} diff --git a/pkg/apis/runtime/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/runtime/v1alpha1/zz_generated.deepcopy.go index a7dbb4d4..7f03a29d 100644 --- a/pkg/apis/runtime/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/runtime/v1alpha1/zz_generated.deepcopy.go @@ -22,6 +22,7 @@ package v1alpha1 import ( corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -246,6 +247,11 @@ func (in *CodeInterpreterSandboxTemplate) DeepCopyInto(out *CodeInterpreterSandb copy(*out, *in) } in.Resources.DeepCopyInto(&out.Resources) + if in.NetworkPolicy != nil { + in, out := &in.NetworkPolicy, &out.NetworkPolicy + *out = new(SandboxNetworkPolicy) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CodeInterpreterSandboxTemplate. @@ -320,6 +326,149 @@ func (in *CodeInterpreterStatus) DeepCopy() *CodeInterpreterStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SandboxEgressRule) DeepCopyInto(out *SandboxEgressRule) { + *out = *in + if in.CIDRs != nil { + in, out := &in.CIDRs, &out.CIDRs + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.Ports != nil { + in, out := &in.Ports, &out.Ports + *out = make([]SandboxNetworkPolicyPort, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SandboxEgressRule. +func (in *SandboxEgressRule) DeepCopy() *SandboxEgressRule { + if in == nil { + return nil + } + out := new(SandboxEgressRule) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SandboxIngressRule) DeepCopyInto(out *SandboxIngressRule) { + *out = *in + if in.CIDRs != nil { + in, out := &in.CIDRs, &out.CIDRs + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.Ports != nil { + in, out := &in.Ports, &out.Ports + *out = make([]SandboxNetworkPolicyPort, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SandboxIngressRule. +func (in *SandboxIngressRule) DeepCopy() *SandboxIngressRule { + if in == nil { + return nil + } + out := new(SandboxIngressRule) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SandboxNetworkPolicy) DeepCopyInto(out *SandboxNetworkPolicy) { + *out = *in + if in.AllowedEgress != nil { + in, out := &in.AllowedEgress, &out.AllowedEgress + *out = make([]SandboxEgressRule, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + if in.AllowDNS != nil { + in, out := &in.AllowDNS, &out.AllowDNS + *out = new(bool) + **out = **in + } + if in.AllowedIngress != nil { + in, out := &in.AllowedIngress, &out.AllowedIngress + *out = make([]SandboxIngressRule, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + if in.Custom != nil { + in, out := &in.Custom, &out.Custom + *out = new(SandboxNetworkPolicyCustomRules) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SandboxNetworkPolicy. +func (in *SandboxNetworkPolicy) DeepCopy() *SandboxNetworkPolicy { + if in == nil { + return nil + } + out := new(SandboxNetworkPolicy) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SandboxNetworkPolicyCustomRules) DeepCopyInto(out *SandboxNetworkPolicyCustomRules) { + *out = *in + if in.Ingress != nil { + in, out := &in.Ingress, &out.Ingress + *out = make([]networkingv1.NetworkPolicyIngressRule, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + if in.Egress != nil { + in, out := &in.Egress, &out.Egress + *out = make([]networkingv1.NetworkPolicyEgressRule, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SandboxNetworkPolicyCustomRules. +func (in *SandboxNetworkPolicyCustomRules) DeepCopy() *SandboxNetworkPolicyCustomRules { + if in == nil { + return nil + } + out := new(SandboxNetworkPolicyCustomRules) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SandboxNetworkPolicyPort) DeepCopyInto(out *SandboxNetworkPolicyPort) { + *out = *in + if in.Protocol != nil { + in, out := &in.Protocol, &out.Protocol + *out = new(corev1.Protocol) + **out = **in + } + out.Port = in.Port +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SandboxNetworkPolicyPort. +func (in *SandboxNetworkPolicyPort) DeepCopy() *SandboxNetworkPolicyPort { + if in == nil { + return nil + } + out := new(SandboxNetworkPolicyPort) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SandboxTemplate) DeepCopyInto(out *SandboxTemplate) { *out = *in @@ -338,6 +487,11 @@ func (in *SandboxTemplate) DeepCopyInto(out *SandboxTemplate) { } } in.Spec.DeepCopyInto(&out.Spec) + if in.NetworkPolicy != nil { + in, out := &in.NetworkPolicy, &out.NetworkPolicy + *out = new(SandboxNetworkPolicy) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SandboxTemplate. diff --git a/pkg/common/types/sandbox.go b/pkg/common/types/sandbox.go index efe16e84..7286df0f 100644 --- a/pkg/common/types/sandbox.go +++ b/pkg/common/types/sandbox.go @@ -21,6 +21,7 @@ import ( "time" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + runtimev1alpha1 "github.com/volcano-sh/agentcube/pkg/apis/runtime/v1alpha1" ) type SandboxInfo struct { @@ -54,6 +55,10 @@ type CreateSandboxRequest struct { Kind string `json:"kind"` Name string `json:"name"` Namespace string `json:"namespace"` + // NetworkPolicy overrides the NetworkPolicy spec from the template for this session only. + // When set, it replaces (not merges) the template-level NetworkPolicy entirely. + // +optional + NetworkPolicy *runtimev1alpha1.SandboxNetworkPolicy `json:"networkPolicy,omitempty"` } type CreateSandboxResponse struct { diff --git a/pkg/workloadmanager/garbage_collection.go b/pkg/workloadmanager/garbage_collection.go index 9353976c..68398a2e 100644 --- a/pkg/workloadmanager/garbage_collection.go +++ b/pkg/workloadmanager/garbage_collection.go @@ -143,22 +143,27 @@ func (gc *garbageCollector) once() { func (gc *garbageCollector) deleteSandbox(ctx context.Context, namespace, name string) error { err := gc.k8sClient.dynamicClient.Resource(SandboxGVR).Namespace(namespace).Delete(ctx, name, metav1.DeleteOptions{}) - if err != nil { - if errors.IsNotFound(err) { - return nil - } + if err != nil && !errors.IsNotFound(err) { return fmt.Errorf("error deleting sandbox %s/%s: %w", namespace, name, err) } + // Best-effort NP cleanup. MUST run even when the Sandbox is already NotFound, + // otherwise an NP orphaned by an out-of-band sandbox delete (e.g. kubectl) + // would leak forever — the store row gets purged right after this call and + // no future GC cycle would see the sandbox again. + if err := deleteNetworkPolicy(ctx, gc.k8sClient.clientset, namespace, name); err != nil { + klog.Infof("garbage collector: network policy %s/%s deletion failed (non-fatal): %v", namespace, name, err) + } return nil } func (gc *garbageCollector) deleteSandboxClaim(ctx context.Context, namespace, name string) error { err := gc.k8sClient.dynamicClient.Resource(SandboxClaimGVR).Namespace(namespace).Delete(ctx, name, metav1.DeleteOptions{}) - if err != nil { - if errors.IsNotFound(err) { - return nil - } + if err != nil && !errors.IsNotFound(err) { return fmt.Errorf("error deleting sandboxClaim %s/%s: %w", namespace, name, err) } + // See deleteSandbox above for why this runs even after a NotFound. + if err := deleteNetworkPolicy(ctx, gc.k8sClient.clientset, namespace, name); err != nil { + klog.Infof("garbage collector: network policy %s/%s deletion failed (non-fatal): %v", namespace, name, err) + } return nil } diff --git a/pkg/workloadmanager/garbage_collection_test.go b/pkg/workloadmanager/garbage_collection_test.go index 531e9908..3e6cf7a5 100644 --- a/pkg/workloadmanager/garbage_collection_test.go +++ b/pkg/workloadmanager/garbage_collection_test.go @@ -26,6 +26,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" dynamicfake "k8s.io/client-go/dynamic/fake" + "k8s.io/client-go/kubernetes/fake" "github.com/volcano-sh/agentcube/pkg/common/types" "github.com/volcano-sh/agentcube/pkg/store" @@ -86,14 +87,16 @@ func (f *gcFakeStore) DeleteSandboxBySessionID(_ context.Context, sessionID stri return nil } -// newTestGC builds a garbageCollector backed by a fake dynamic client and the -// provided store. The fake dynamic client returns "not found" for all deletes -// (no pre-loaded objects), which garbageCollector treats as success. +// newTestGC builds a garbageCollector backed by fake dynamic and clientset clients +// and the provided store. Both fakes return "not found" for all deletes (no pre-loaded +// objects), which garbageCollector treats as success. The clientset is needed so the +// NetworkPolicy cleanup path in deleteSandbox/deleteSandboxClaim does not nil-panic. func newTestGC(s store.Store) *garbageCollector { scheme := runtime.NewScheme() fakeDynamic := dynamicfake.NewSimpleDynamicClient(scheme) + fakeClientset := fake.NewSimpleClientset() return &garbageCollector{ - k8sClient: &K8sClient{dynamicClient: fakeDynamic}, + k8sClient: &K8sClient{dynamicClient: fakeDynamic, clientset: fakeClientset}, storeClient: s, interval: time.Minute, } diff --git a/pkg/workloadmanager/handlers.go b/pkg/workloadmanager/handlers.go index 7072b43b..1d3b1fed 100644 --- a/pkg/workloadmanager/handlers.go +++ b/pkg/workloadmanager/handlers.go @@ -24,6 +24,7 @@ import ( "time" "github.com/gin-gonic/gin" + networkingv1 "k8s.io/api/networking/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/client-go/dynamic" "k8s.io/klog/v2" @@ -90,13 +91,14 @@ func (s *Server) handleSandboxCreate(c *gin.Context, kind string) { var sandbox *sandboxv1alpha1.Sandbox var sandboxClaim *extensionsv1alpha1.SandboxClaim + var networkPolicy *networkingv1.NetworkPolicy var sandboxEntry *sandboxEntry var err error switch sandboxReq.Kind { case types.AgentRuntimeKind: - sandbox, sandboxEntry, err = buildSandboxByAgentRuntime(sandboxReq.Namespace, sandboxReq.Name, s.informers) + sandbox, sandboxEntry, networkPolicy, err = buildSandboxByAgentRuntime(sandboxReq.Namespace, sandboxReq.Name, s.informers, s.config.RouterSelector, s.config.RouterNamespace, sandboxReq.NetworkPolicy) case types.CodeInterpreterKind: - sandbox, sandboxClaim, sandboxEntry, err = buildSandboxByCodeInterpreter(sandboxReq.Namespace, sandboxReq.Name, s.informers) + sandbox, sandboxClaim, networkPolicy, sandboxEntry, err = buildSandboxByCodeInterpreter(sandboxReq.Namespace, sandboxReq.Name, s.informers, s.config.RouterSelector, s.config.RouterNamespace, sandboxReq.NetworkPolicy) } if err != nil { @@ -130,7 +132,7 @@ func (s *Server) handleSandboxCreate(c *gin.Context, kind string) { // Ensure cleanup is called when function returns to prevent memory leak defer s.sandboxController.UnWatchSandbox(namespace, sandboxName) - response, err := s.createSandbox(c.Request.Context(), dynamicClient, sandbox, sandboxClaim, sandboxEntry, resultChan) + response, err := s.createSandbox(c.Request.Context(), dynamicClient, sandbox, sandboxClaim, networkPolicy, sandboxEntry, resultChan) if err != nil { klog.Errorf("create sandbox failed %s/%s: %v", sandbox.Namespace, sandbox.Name, err) respondError(c, http.StatusInternalServerError, "internal server error") @@ -141,7 +143,7 @@ func (s *Server) handleSandboxCreate(c *gin.Context, kind string) { } // createSandbox performs sandbox creation and returns the response payload or an error with an HTTP status code. -func (s *Server) createSandbox(ctx context.Context, dynamicClient dynamic.Interface, sandbox *sandboxv1alpha1.Sandbox, sandboxClaim *extensionsv1alpha1.SandboxClaim, sandboxEntry *sandboxEntry, resultChan <-chan SandboxStatusUpdate) (*types.CreateSandboxResponse, error) { +func (s *Server) createSandbox(ctx context.Context, dynamicClient dynamic.Interface, sandbox *sandboxv1alpha1.Sandbox, sandboxClaim *extensionsv1alpha1.SandboxClaim, networkPolicy *networkingv1.NetworkPolicy, sandboxEntry *sandboxEntry, resultChan <-chan SandboxStatusUpdate) (*types.CreateSandboxResponse, error) { // Store placeholder before creating, make sandbox/sandboxClaim GarbageCollection possible sandboxStorePlaceHolder := buildSandboxPlaceHolder(sandbox, sandboxEntry) if err := s.storeClient.StoreSandbox(ctx, sandboxStorePlaceHolder); err != nil { @@ -160,17 +162,28 @@ func (s *Server) createSandbox(ctx context.Context, dynamicClient dynamic.Interf } } - // Register rollback BEFORE waiting for the sandbox to become ready. - // This ensures the K8s resource and store placeholder are cleaned up on - // timeout, pod-IP failure, or store-update failure — not just on post-creation errors. + // Register rollback IMMEDIATELY after the sandbox/claim exists, before any + // subsequent failure path (NetworkPolicy create, ready wait, entrypoint probe, + // store update). Rollback unconditionally attempts NP delete when networkPolicy + // is non-nil; delete is idempotent (NotFound is swallowed), which also covers + // the edge case where Create succeeded server-side but returned an error + // (network timeout after write) — the NP exists and rollback reaps it. needRollbackSandbox := true defer func() { if !needRollbackSandbox { return } - s.sandboxRollback(sandboxClaim, dynamicClient, sandbox, sandboxEntry) + s.sandboxRollback(sandboxClaim, dynamicClient, sandbox, networkPolicy, sandboxEntry) }() + // Create NetworkPolicy if one was built from the template spec. + // Uses the workloadmanager's own client (not the user client) since NP is infra. + if networkPolicy != nil { + if err := createNetworkPolicy(ctx, s.k8sClient.clientset, networkPolicy); err != nil { + return nil, api.NewInternalError(err) + } + } + var createdSandbox *sandboxv1alpha1.Sandbox select { case result := <-resultChan: @@ -217,7 +230,7 @@ func (s *Server) createSandbox(ctx context.Context, dynamicClient dynamic.Interf return response, nil } -func (s *Server) sandboxRollback(sandboxClaim *extensionsv1alpha1.SandboxClaim, dynamicClient dynamic.Interface, sandbox *sandboxv1alpha1.Sandbox, sandboxEntry *sandboxEntry) { +func (s *Server) sandboxRollback(sandboxClaim *extensionsv1alpha1.SandboxClaim, dynamicClient dynamic.Interface, sandbox *sandboxv1alpha1.Sandbox, networkPolicy *networkingv1.NetworkPolicy, sandboxEntry *sandboxEntry) { ctxTimeout, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() var err error @@ -234,6 +247,14 @@ func (s *Server) sandboxRollback(sandboxClaim *extensionsv1alpha1.SandboxClaim, klog.Infof("sandbox %s/%s rollback failed: %v", sandbox.Namespace, sandbox.Name, err) } } + // Always attempt to delete the NetworkPolicy when one was built. deleteNetworkPolicy + // swallows NotFound, so this handles both "create succeeded" and "create returned + // an error despite server-side success" uniformly. + if networkPolicy != nil { + if err := deleteNetworkPolicy(ctxTimeout, s.k8sClient.clientset, networkPolicy.Namespace, networkPolicy.Name); err != nil { + klog.Infof("network policy %s/%s rollback failed: %v", networkPolicy.Namespace, networkPolicy.Name, err) + } + } // Clean up the store placeholder so it does not pollute GC queries if delErr := s.storeClient.DeleteSandboxBySessionID(ctxTimeout, sandboxEntry.SessionID); delErr != nil { klog.Infof("sandbox %s/%s store placeholder cleanup failed: %v", sandbox.Namespace, sandbox.Name, delErr) @@ -291,6 +312,12 @@ func (s *Server) handleDeleteSandbox(c *gin.Context) { } } + // Best-effort deletion of the NetworkPolicy that was created alongside this sandbox. + // The NP shares the sandbox name, so not-found is expected when Mode=None. + if err := deleteNetworkPolicy(c.Request.Context(), s.k8sClient.clientset, sandbox.SandboxNamespace, sandbox.Name); err != nil { + klog.Infof("network policy %s/%s deletion failed (non-fatal): %v", sandbox.SandboxNamespace, sandbox.Name, err) + } + // Delete sandbox from store err = s.storeClient.DeleteSandboxBySessionID(c.Request.Context(), sessionID) if err != nil { diff --git a/pkg/workloadmanager/handlers_test.go b/pkg/workloadmanager/handlers_test.go index ec8f8b05..a49aef63 100644 --- a/pkg/workloadmanager/handlers_test.go +++ b/pkg/workloadmanager/handlers_test.go @@ -29,11 +29,13 @@ import ( "github.com/agiledragon/gomonkey/v2" "github.com/gin-gonic/gin" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/volcano-sh/agentcube/pkg/api" runtimev1alpha1 "github.com/volcano-sh/agentcube/pkg/apis/runtime/v1alpha1" "github.com/volcano-sh/agentcube/pkg/common/types" "github.com/volcano-sh/agentcube/pkg/store" + networkingv1 "k8s.io/api/networking/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/dynamic" @@ -231,7 +233,7 @@ func TestServerCreateSandbox(t *testing.T) { return tt.readyErr }) - resp, err := server.createSandbox(context.Background(), nil, sb, claim, makeEntry(), resultChan) + resp, err := server.createSandbox(context.Background(), nil, sb, claim, nil, makeEntry(), resultChan) require.Equal(t, tt.expectCreateCalls, createCalls, "createSandbox call count") require.Equal(t, tt.expectClaimCalls, claimCalls, "createSandboxClaim call count") @@ -375,28 +377,28 @@ func TestHandleSandboxCreate(t *testing.T) { patches := gomonkey.NewPatches() defer patches.Reset() - patches.ApplyFunc(buildSandboxByAgentRuntime, func(_, _ string, _ *Informers) (*sandboxv1alpha1.Sandbox, *sandboxEntry, error) { + patches.ApplyFunc(buildSandboxByAgentRuntime, func(_, _ string, _ *Informers, _ map[string]string, _ string, _ *runtimev1alpha1.SandboxNetworkPolicy) (*sandboxv1alpha1.Sandbox, *sandboxEntry, *networkingv1.NetworkPolicy, error) { if tc.kind != types.AgentRuntimeKind { - return nil, nil, errors.New("unexpected kind") + return nil, nil, nil, errors.New("unexpected kind") } if tc.buildErr != nil { - return nil, nil, tc.buildErr + return nil, nil, nil, tc.buildErr } - return sb, entry, nil + return sb, entry, nil, nil }) - patches.ApplyFunc(buildSandboxByCodeInterpreter, func(_, _ string, _ *Informers) (*sandboxv1alpha1.Sandbox, *extensionsv1alpha1.SandboxClaim, *sandboxEntry, error) { + patches.ApplyFunc(buildSandboxByCodeInterpreter, func(_, _ string, _ *Informers, _ map[string]string, _ string, _ *runtimev1alpha1.SandboxNetworkPolicy) (*sandboxv1alpha1.Sandbox, *extensionsv1alpha1.SandboxClaim, *networkingv1.NetworkPolicy, *sandboxEntry, error) { if tc.kind != types.CodeInterpreterKind { - return nil, nil, nil, errors.New("unexpected kind") + return nil, nil, nil, nil, errors.New("unexpected kind") } if tc.buildErr != nil { - return nil, nil, nil, tc.buildErr + return nil, nil, nil, nil, tc.buildErr } - return sb, claim, entry, nil + return sb, claim, nil, entry, nil }) createCalls := 0 - patches.ApplyPrivateMethod(reflect.TypeOf(fakeServer), "createSandbox", func(_ *Server, _ context.Context, _ dynamic.Interface, _ *sandboxv1alpha1.Sandbox, _ *extensionsv1alpha1.SandboxClaim, _ *sandboxEntry, _ <-chan SandboxStatusUpdate) (*types.CreateSandboxResponse, error) { + patches.ApplyPrivateMethod(reflect.TypeOf(fakeServer), "createSandbox", func(_ *Server, _ context.Context, _ dynamic.Interface, _ *sandboxv1alpha1.Sandbox, _ *extensionsv1alpha1.SandboxClaim, _ *networkingv1.NetworkPolicy, _ *sandboxEntry, _ <-chan SandboxStatusUpdate) (*types.CreateSandboxResponse, error) { createCalls++ if tc.createErr != nil { return nil, tc.createErr @@ -427,3 +429,51 @@ func TestHandleSandboxCreate(t *testing.T) { }) } } + +// TestHandleSandboxCreate_SessionNetworkPolicyOverride verifies that a per-session +// NetworkPolicy override in the request body is forwarded unchanged to the builder. +// This is the "replace, not merge" contract promised to the maintainer. +func TestHandleSandboxCreate_SessionNetworkPolicyOverride(t *testing.T) { + gin.SetMode(gin.TestMode) + + body := `{"name":"workload","namespace":"ns","networkPolicy":{"mode":"Restricted","allowDNS":false}}` + + for _, kind := range []string{types.AgentRuntimeKind, types.CodeInterpreterKind} { + t.Run(kind, func(t *testing.T) { + fakeServer := newFakeServer() + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + + req := httptest.NewRequest(http.MethodPost, "/", bytes.NewBufferString(body)) + req.Header.Set("Content-Type", "application/json") + c.Request = req + + sb, entry := makeSandbox(kind, "ns", "sandbox-1") + claim := &extensionsv1alpha1.SandboxClaim{ObjectMeta: metav1.ObjectMeta{Name: sb.Name, Namespace: sb.Namespace}} + + patches := gomonkey.NewPatches() + defer patches.Reset() + + var capturedNP *runtimev1alpha1.SandboxNetworkPolicy + patches.ApplyFunc(buildSandboxByAgentRuntime, func(_, _ string, _ *Informers, _ map[string]string, _ string, sessionNP *runtimev1alpha1.SandboxNetworkPolicy) (*sandboxv1alpha1.Sandbox, *sandboxEntry, *networkingv1.NetworkPolicy, error) { + capturedNP = sessionNP + return sb, entry, nil, nil + }) + patches.ApplyFunc(buildSandboxByCodeInterpreter, func(_, _ string, _ *Informers, _ map[string]string, _ string, sessionNP *runtimev1alpha1.SandboxNetworkPolicy) (*sandboxv1alpha1.Sandbox, *extensionsv1alpha1.SandboxClaim, *networkingv1.NetworkPolicy, *sandboxEntry, error) { + capturedNP = sessionNP + return sb, claim, nil, entry, nil + }) + patches.ApplyPrivateMethod(reflect.TypeOf(fakeServer), "createSandbox", func(_ *Server, _ context.Context, _ dynamic.Interface, _ *sandboxv1alpha1.Sandbox, _ *extensionsv1alpha1.SandboxClaim, _ *networkingv1.NetworkPolicy, _ *sandboxEntry, _ <-chan SandboxStatusUpdate) (*types.CreateSandboxResponse, error) { + return &types.CreateSandboxResponse{SessionID: "sess-1"}, nil + }) + + fakeServer.handleSandboxCreate(c, kind) + + require.Equal(t, http.StatusOK, w.Code) + require.NotNil(t, capturedNP, "session NetworkPolicy override must reach the builder") + assert.Equal(t, runtimev1alpha1.NetworkPolicyModeRestricted, capturedNP.Mode) + require.NotNil(t, capturedNP.AllowDNS) + assert.False(t, *capturedNP.AllowDNS, "AllowDNS=false from request body should propagate") + }) + } +} diff --git a/pkg/workloadmanager/k8s_client.go b/pkg/workloadmanager/k8s_client.go index 15cc47bd..6aa3483c 100644 --- a/pkg/workloadmanager/k8s_client.go +++ b/pkg/workloadmanager/k8s_client.go @@ -25,6 +25,7 @@ import ( runtimev1alpha1 "github.com/volcano-sh/agentcube/pkg/apis/runtime/v1alpha1" corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -62,7 +63,7 @@ var ( // K8sClient encapsulates the Kubernetes client type K8sClient struct { - clientset *kubernetes.Clientset + clientset kubernetes.Interface dynamicClient dynamic.Interface scheme *runtime.Scheme baseConfig *rest.Config // Store base config for creating user clients @@ -383,3 +384,21 @@ func (c *K8sClient) WaitForSandboxReady(ctx context.Context, namespace, sandboxN func (c *K8sClient) GetSandboxInformer() cache.SharedInformer { return c.dynamicInformer.ForResource(SandboxGVR).Informer() } + +// createNetworkPolicy creates a NetworkPolicy using the workloadmanager's own client. +func createNetworkPolicy(ctx context.Context, clientset kubernetes.Interface, np *networkingv1.NetworkPolicy) error { + _, err := clientset.NetworkingV1().NetworkPolicies(np.Namespace).Create(ctx, np, metav1.CreateOptions{}) + if err != nil && !apierrors.IsAlreadyExists(err) { + return fmt.Errorf("failed to create network policy %s/%s: %w", np.Namespace, np.Name, err) + } + return nil +} + +// deleteNetworkPolicy deletes the NetworkPolicy with the given name, ignoring not-found. +func deleteNetworkPolicy(ctx context.Context, clientset kubernetes.Interface, namespace, name string) error { + err := clientset.NetworkingV1().NetworkPolicies(namespace).Delete(ctx, name, metav1.DeleteOptions{}) + if err != nil && !apierrors.IsNotFound(err) { + return fmt.Errorf("failed to delete network policy %s/%s: %w", namespace, name, err) + } + return nil +} diff --git a/pkg/workloadmanager/networkpolicy_builder.go b/pkg/workloadmanager/networkpolicy_builder.go new file mode 100644 index 00000000..f77dd3d8 --- /dev/null +++ b/pkg/workloadmanager/networkpolicy_builder.go @@ -0,0 +1,149 @@ +/* +Copyright The Volcano Authors. + +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 workloadmanager + +import ( + runtimev1alpha1 "github.com/volcano-sh/agentcube/pkg/apis/runtime/v1alpha1" + corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" +) + +// k8sNamespaceLabelKey is the label automatically applied to every Namespace by +// Kubernetes 1.22+ containing the namespace's own name. Using it as a +// NamespaceSelector key avoids requiring users to manually label namespaces. +const k8sNamespaceLabelKey = "kubernetes.io/metadata.name" + +// buildNetworkPolicy returns a NetworkPolicy for the given sandbox, or nil when Mode=None. +func buildNetworkPolicy(sandboxName, namespace string, spec *runtimev1alpha1.SandboxNetworkPolicy, routerSelector map[string]string, routerNamespace string) *networkingv1.NetworkPolicy { + if spec == nil || spec.Mode == runtimev1alpha1.NetworkPolicyModeNone || spec.Mode == "" { + return nil + } + + np := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: sandboxName, + Namespace: namespace, + Labels: map[string]string{ + SandboxNameLabelKey: sandboxName, + "managed-by": "agentcube-workload-manager", + }, + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + SandboxNameLabelKey: sandboxName, + }, + }, + PolicyTypes: []networkingv1.PolicyType{ + networkingv1.PolicyTypeIngress, + networkingv1.PolicyTypeEgress, + }, + }, + } + + switch spec.Mode { + case runtimev1alpha1.NetworkPolicyModeRestricted: + np.Spec.Ingress = buildRestrictedIngressRules(spec, routerSelector, routerNamespace) + np.Spec.Egress = buildRestrictedEgressRules(spec) + case runtimev1alpha1.NetworkPolicyModeCustom: + if spec.Custom != nil { + np.Spec.Ingress = spec.Custom.Ingress + np.Spec.Egress = spec.Custom.Egress + } + } + + return np +} + +func buildRestrictedIngressRules(spec *runtimev1alpha1.SandboxNetworkPolicy, routerSelector map[string]string, routerNamespace string) []networkingv1.NetworkPolicyIngressRule { + // Router ingress is always allowed in Restricted mode. NamespaceSelector is set + // so the rule works even when the router runs in a namespace distinct from the + // sandbox (the common production layout: router in agentcube-system, sandboxes + // in user namespaces). + routerPeer := networkingv1.NetworkPolicyPeer{ + PodSelector: &metav1.LabelSelector{MatchLabels: routerSelector}, + } + if routerNamespace != "" { + routerPeer.NamespaceSelector = &metav1.LabelSelector{ + MatchLabels: map[string]string{k8sNamespaceLabelKey: routerNamespace}, + } + } + rules := []networkingv1.NetworkPolicyIngressRule{ + {From: []networkingv1.NetworkPolicyPeer{routerPeer}}, + } + for _, r := range spec.AllowedIngress { + rule := networkingv1.NetworkPolicyIngressRule{} + for _, cidr := range r.CIDRs { + rule.From = append(rule.From, networkingv1.NetworkPolicyPeer{ + IPBlock: &networkingv1.IPBlock{CIDR: cidr}, + }) + } + rule.Ports = toNetworkPolicyPorts(r.Ports) + rules = append(rules, rule) + } + return rules +} + +func buildRestrictedEgressRules(spec *runtimev1alpha1.SandboxNetworkPolicy) []networkingv1.NetworkPolicyEgressRule { + var rules []networkingv1.NetworkPolicyEgressRule + + // Allow DNS by default (unless explicitly disabled). + if spec.AllowDNS == nil || *spec.AllowDNS { + tcp := corev1.ProtocolTCP + udp := corev1.ProtocolUDP + dnsPort := intstr.FromInt32(53) + rules = append(rules, networkingv1.NetworkPolicyEgressRule{ + Ports: []networkingv1.NetworkPolicyPort{ + {Protocol: &udp, Port: &dnsPort}, + {Protocol: &tcp, Port: &dnsPort}, + }, + }) + } + + for _, r := range spec.AllowedEgress { + rule := networkingv1.NetworkPolicyEgressRule{} + for _, cidr := range r.CIDRs { + rule.To = append(rule.To, networkingv1.NetworkPolicyPeer{ + IPBlock: &networkingv1.IPBlock{CIDR: cidr}, + }) + } + rule.Ports = toNetworkPolicyPorts(r.Ports) + rules = append(rules, rule) + } + return rules +} + +func toNetworkPolicyPorts(ports []runtimev1alpha1.SandboxNetworkPolicyPort) []networkingv1.NetworkPolicyPort { + if len(ports) == 0 { + return nil + } + out := make([]networkingv1.NetworkPolicyPort, 0, len(ports)) + for i := range ports { + // Copy values rather than aliasing pointers into the user's spec, so later + // mutation of the source slice cannot affect the generated NetworkPolicy. + port := ports[i].Port + entry := networkingv1.NetworkPolicyPort{Port: &port} + if ports[i].Protocol != nil { + proto := *ports[i].Protocol + entry.Protocol = &proto + } + out = append(out, entry) + } + return out +} diff --git a/pkg/workloadmanager/networkpolicy_builder_test.go b/pkg/workloadmanager/networkpolicy_builder_test.go new file mode 100644 index 00000000..89679379 --- /dev/null +++ b/pkg/workloadmanager/networkpolicy_builder_test.go @@ -0,0 +1,271 @@ +/* +Copyright The Volcano Authors. + +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 workloadmanager + +import ( + "testing" + + runtimev1alpha1 "github.com/volcano-sh/agentcube/pkg/apis/runtime/v1alpha1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/ptr" +) + +var ( + testRouterSelector = map[string]string{"app": "agentcube-router"} + testRouterNamespace = "agentcube-system" +) + +func TestBuildNetworkPolicy_NoneMode(t *testing.T) { + // nil spec → no NP + np := buildNetworkPolicy("sandbox-1", "default", nil, testRouterSelector, testRouterNamespace) + assert.Nil(t, np) + + // explicit None → no NP + np = buildNetworkPolicy("sandbox-1", "default", &runtimev1alpha1.SandboxNetworkPolicy{ + Mode: runtimev1alpha1.NetworkPolicyModeNone, + }, testRouterSelector, testRouterNamespace) + assert.Nil(t, np) + + // empty mode string → no NP + np = buildNetworkPolicy("sandbox-1", "default", &runtimev1alpha1.SandboxNetworkPolicy{}, testRouterSelector, testRouterNamespace) + assert.Nil(t, np) +} + +func TestBuildNetworkPolicy_Metadata(t *testing.T) { + np := buildNetworkPolicy("my-sandbox", "my-ns", &runtimev1alpha1.SandboxNetworkPolicy{ + Mode: runtimev1alpha1.NetworkPolicyModeRestricted, + }, testRouterSelector, testRouterNamespace) + require.NotNil(t, np) + + assert.Equal(t, "my-sandbox", np.Name) + assert.Equal(t, "my-ns", np.Namespace) + assert.Equal(t, "my-sandbox", np.Labels[SandboxNameLabelKey]) + assert.Equal(t, map[string]string{SandboxNameLabelKey: "my-sandbox"}, np.Spec.PodSelector.MatchLabels) + assert.Contains(t, np.Spec.PolicyTypes, networkingv1.PolicyTypeIngress) + assert.Contains(t, np.Spec.PolicyTypes, networkingv1.PolicyTypeEgress) +} + +func TestBuildNetworkPolicy_RestrictedDefaults(t *testing.T) { + np := buildNetworkPolicy("sb", "ns", &runtimev1alpha1.SandboxNetworkPolicy{ + Mode: runtimev1alpha1.NetworkPolicyModeRestricted, + }, testRouterSelector, testRouterNamespace) + require.NotNil(t, np) + + // Ingress: only router rule by default + require.Len(t, np.Spec.Ingress, 1) + require.Len(t, np.Spec.Ingress[0].From, 1) + assert.Equal(t, testRouterSelector, np.Spec.Ingress[0].From[0].PodSelector.MatchLabels) + + // Egress: DNS rule (UDP+TCP 53) by default + require.Len(t, np.Spec.Egress, 1) + dnsRule := np.Spec.Egress[0] + assert.Empty(t, dnsRule.To) // unrestricted destination, only port-limited + require.Len(t, dnsRule.Ports, 2) + ports := map[corev1.Protocol]bool{} + for _, p := range dnsRule.Ports { + require.NotNil(t, p.Protocol) + assert.Equal(t, intstr.FromInt32(53), *p.Port) + ports[*p.Protocol] = true + } + assert.True(t, ports[corev1.ProtocolUDP], "expected UDP DNS port") + assert.True(t, ports[corev1.ProtocolTCP], "expected TCP DNS port") +} + +func TestBuildNetworkPolicy_RestrictedDNSDisabled(t *testing.T) { + np := buildNetworkPolicy("sb", "ns", &runtimev1alpha1.SandboxNetworkPolicy{ + Mode: runtimev1alpha1.NetworkPolicyModeRestricted, + AllowDNS: ptr.To(false), + }, testRouterSelector, testRouterNamespace) + require.NotNil(t, np) + assert.Empty(t, np.Spec.Egress, "expected no egress rules when DNS disabled and no AllowedEgress") +} + +func TestBuildNetworkPolicy_RestrictedAllowedEgress(t *testing.T) { + tcp := corev1.ProtocolTCP + np := buildNetworkPolicy("sb", "ns", &runtimev1alpha1.SandboxNetworkPolicy{ + Mode: runtimev1alpha1.NetworkPolicyModeRestricted, + AllowedEgress: []runtimev1alpha1.SandboxEgressRule{ + { + CIDRs: []string{"10.0.0.0/8"}, + Ports: []runtimev1alpha1.SandboxNetworkPolicyPort{ + {Protocol: &tcp, Port: intstr.FromInt32(443)}, + }, + }, + }, + }, testRouterSelector, testRouterNamespace) + require.NotNil(t, np) + + // DNS rule (index 0) + custom egress rule (index 1) + require.Len(t, np.Spec.Egress, 2) + customRule := np.Spec.Egress[1] + require.Len(t, customRule.To, 1) + assert.Equal(t, "10.0.0.0/8", customRule.To[0].IPBlock.CIDR) + require.Len(t, customRule.Ports, 1) + assert.Equal(t, intstr.FromInt32(443), *customRule.Ports[0].Port) + assert.Equal(t, corev1.ProtocolTCP, *customRule.Ports[0].Protocol) +} + +func TestBuildNetworkPolicy_RestrictedAllowedIngress(t *testing.T) { + np := buildNetworkPolicy("sb", "ns", &runtimev1alpha1.SandboxNetworkPolicy{ + Mode: runtimev1alpha1.NetworkPolicyModeRestricted, + AllowedIngress: []runtimev1alpha1.SandboxIngressRule{ + {CIDRs: []string{"192.168.1.0/24"}}, + }, + }, testRouterSelector, testRouterNamespace) + require.NotNil(t, np) + + // router rule (index 0) + custom ingress (index 1) + require.Len(t, np.Spec.Ingress, 2) + customRule := np.Spec.Ingress[1] + require.Len(t, customRule.From, 1) + assert.Equal(t, "192.168.1.0/24", customRule.From[0].IPBlock.CIDR) +} + +func TestBuildNetworkPolicy_CustomMode(t *testing.T) { + tcp := corev1.ProtocolTCP + port80 := intstr.FromInt32(80) + np := buildNetworkPolicy("sb", "ns", &runtimev1alpha1.SandboxNetworkPolicy{ + Mode: runtimev1alpha1.NetworkPolicyModeCustom, + Custom: &runtimev1alpha1.SandboxNetworkPolicyCustomRules{ + Ingress: []networkingv1.NetworkPolicyIngressRule{ + { + From: []networkingv1.NetworkPolicyPeer{{IPBlock: &networkingv1.IPBlock{CIDR: "0.0.0.0/0"}}}, + Ports: []networkingv1.NetworkPolicyPort{{Protocol: &tcp, Port: &port80}}, + }, + }, + Egress: []networkingv1.NetworkPolicyEgressRule{ + {To: []networkingv1.NetworkPolicyPeer{{IPBlock: &networkingv1.IPBlock{CIDR: "0.0.0.0/0"}}}}, + }, + }, + }, testRouterSelector, testRouterNamespace) + require.NotNil(t, np) + + require.Len(t, np.Spec.Ingress, 1) + assert.Equal(t, "0.0.0.0/0", np.Spec.Ingress[0].From[0].IPBlock.CIDR) + require.Len(t, np.Spec.Egress, 1) + assert.Equal(t, "0.0.0.0/0", np.Spec.Egress[0].To[0].IPBlock.CIDR) +} + +func TestBuildNetworkPolicy_CustomModeNilCustom(t *testing.T) { + // Mode=Custom but Custom is nil → NP created with empty rules (deny all) + np := buildNetworkPolicy("sb", "ns", &runtimev1alpha1.SandboxNetworkPolicy{ + Mode: runtimev1alpha1.NetworkPolicyModeCustom, + }, testRouterSelector, testRouterNamespace) + require.NotNil(t, np) + assert.Empty(t, np.Spec.Ingress) + assert.Empty(t, np.Spec.Egress) +} + +func TestBuildNetworkPolicy_RouterSelector(t *testing.T) { + customSelector := map[string]string{"app": "my-custom-router", "tier": "proxy"} + np := buildNetworkPolicy("sb", "ns", &runtimev1alpha1.SandboxNetworkPolicy{ + Mode: runtimev1alpha1.NetworkPolicyModeRestricted, + }, customSelector, testRouterNamespace) + require.NotNil(t, np) + + require.Len(t, np.Spec.Ingress, 1) + ingressPeer := np.Spec.Ingress[0].From[0] + require.NotNil(t, ingressPeer.PodSelector) + assert.Equal(t, customSelector, ingressPeer.PodSelector.MatchLabels) +} + +func TestBuildNetworkPolicy_RouterCrossNamespace(t *testing.T) { + // When routerNamespace is set, the router peer must carry a NamespaceSelector + // using the standard kubernetes.io/metadata.name label so the rule works when + // the sandbox is in a different namespace than the router. + np := buildNetworkPolicy("sb", "user-ns", &runtimev1alpha1.SandboxNetworkPolicy{ + Mode: runtimev1alpha1.NetworkPolicyModeRestricted, + }, testRouterSelector, "agentcube-system") + require.NotNil(t, np) + + require.Len(t, np.Spec.Ingress, 1) + routerPeer := np.Spec.Ingress[0].From[0] + require.NotNil(t, routerPeer.PodSelector) + require.NotNil(t, routerPeer.NamespaceSelector) + assert.Equal(t, map[string]string{k8sNamespaceLabelKey: "agentcube-system"}, routerPeer.NamespaceSelector.MatchLabels) +} + +func TestBuildNetworkPolicy_RouterNamespaceEmpty(t *testing.T) { + // Empty routerNamespace falls back to same-namespace semantics (no NamespaceSelector). + np := buildNetworkPolicy("sb", "ns", &runtimev1alpha1.SandboxNetworkPolicy{ + Mode: runtimev1alpha1.NetworkPolicyModeRestricted, + }, testRouterSelector, "") + require.NotNil(t, np) + + require.Len(t, np.Spec.Ingress, 1) + routerPeer := np.Spec.Ingress[0].From[0] + assert.Nil(t, routerPeer.NamespaceSelector) +} + +func TestBuildNetworkPolicy_SandboxNameLabel(t *testing.T) { + np := buildNetworkPolicy("unique-sandbox", "test-ns", &runtimev1alpha1.SandboxNetworkPolicy{ + Mode: runtimev1alpha1.NetworkPolicyModeRestricted, + }, testRouterSelector, testRouterNamespace) + require.NotNil(t, np) + + // Pod selector must target exactly this sandbox + assert.Equal(t, metav1.LabelSelector{ + MatchLabels: map[string]string{SandboxNameLabelKey: "unique-sandbox"}, + }, np.Spec.PodSelector) +} + +// TestEffectiveNetworkPolicy verifies "replace, not merge" semantics between +// template-level default and per-session override (promised to the maintainer). +func TestEffectiveNetworkPolicy(t *testing.T) { + template := &runtimev1alpha1.SandboxNetworkPolicy{ + Mode: runtimev1alpha1.NetworkPolicyModeRestricted, + AllowedEgress: []runtimev1alpha1.SandboxEgressRule{ + {CIDRs: []string{"10.0.0.0/8"}}, + }, + } + session := &runtimev1alpha1.SandboxNetworkPolicy{ + Mode: runtimev1alpha1.NetworkPolicyModeCustom, + } + + // session nil → template wins + assert.Equal(t, template, effectiveNetworkPolicy(nil, template)) + // session set → replaces template entirely (no merge) + assert.Equal(t, session, effectiveNetworkPolicy(session, template)) + // both nil → nil (no NP) + assert.Nil(t, effectiveNetworkPolicy(nil, nil)) + // template nil, session set → session + assert.Equal(t, session, effectiveNetworkPolicy(session, nil)) +} + +// TestToNetworkPolicyPorts_NoAliasing verifies mutations of the source slice +// after build do not affect the generated NetworkPolicy ports. +func TestToNetworkPolicyPorts_NoAliasing(t *testing.T) { + tcp := corev1.ProtocolTCP + src := []runtimev1alpha1.SandboxNetworkPolicyPort{ + {Protocol: &tcp, Port: intstr.FromInt32(443)}, + } + out := toNetworkPolicyPorts(src) + require.Len(t, out, 1) + + // Mutate the source after the call. + udp := corev1.ProtocolUDP + src[0].Protocol = &udp + src[0].Port = intstr.FromInt32(80) + + assert.Equal(t, corev1.ProtocolTCP, *out[0].Protocol, "output protocol must not track source mutation") + assert.Equal(t, int32(443), out[0].Port.IntVal, "output port must not track source mutation") +} diff --git a/pkg/workloadmanager/server.go b/pkg/workloadmanager/server.go index 53b10dfb..3e9283ac 100644 --- a/pkg/workloadmanager/server.go +++ b/pkg/workloadmanager/server.go @@ -44,6 +44,10 @@ type Server struct { wg sync.WaitGroup } +// DefaultRouterSelector is the pod label selector used to identify the router pod +// when building the mandatory router→sandbox ingress rule in Restricted network policy mode. +var DefaultRouterSelector = map[string]string{"app": "agentcube-router"} + type Config struct { // Port is the port the API server listens on Port string @@ -62,6 +66,15 @@ type Config struct { SandboxReadyProbeTimeout time.Duration // SandboxReadyProbeInterval is the retry interval for sandbox entrypoint probes. SandboxReadyProbeInterval time.Duration + // RouterSelector is the pod label selector identifying the router. + // Used to build the mandatory router→sandbox ingress allow rule when + // SandboxNetworkPolicy.Mode=Restricted. Defaults to DefaultRouterSelector. + RouterSelector map[string]string + // RouterNamespace is the namespace the router runs in. Used to build the + // NamespaceSelector for the router→sandbox ingress rule when sandboxes live + // in a different namespace than the router. Defaults to the workloadmanager's + // own namespace (AGENTCUBE_NAMESPACE). + RouterNamespace string } // NewServer creates a new API server instance @@ -75,6 +88,15 @@ func NewServer(config *Config, sandboxController *SandboxReconciler) (*Server, e if config.SandboxReadyProbeInterval <= 0 { config.SandboxReadyProbeInterval = defaultSandboxReadyProbeInterval } + if len(config.RouterSelector) == 0 { + config.RouterSelector = DefaultRouterSelector + } + if config.RouterNamespace == "" { + // IdentitySecretNamespace is derived from AGENTCUBE_NAMESPACE and is the + // namespace the workloadmanager itself runs in (same as the router in + // standard deployments). + config.RouterNamespace = IdentitySecretNamespace + } // Create Kubernetes client k8sClient, err := NewK8sClient() diff --git a/pkg/workloadmanager/workload_builder.go b/pkg/workloadmanager/workload_builder.go index 9c07c5bb..4d6dc73d 100644 --- a/pkg/workloadmanager/workload_builder.go +++ b/pkg/workloadmanager/workload_builder.go @@ -28,6 +28,7 @@ import ( runtimev1alpha1 "github.com/volcano-sh/agentcube/pkg/apis/runtime/v1alpha1" "github.com/volcano-sh/agentcube/pkg/common/types" corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" @@ -128,6 +129,16 @@ func loadPublicKeyFromSecret(clientset kubernetes.Interface) error { return nil } +// effectiveNetworkPolicy returns the session-level override when set, otherwise the +// template-level default. Override semantics are "replace, not merge": the session +// policy replaces the template policy in its entirety. +func effectiveNetworkPolicy(session, template *runtimev1alpha1.SandboxNetworkPolicy) *runtimev1alpha1.SandboxNetworkPolicy { + if session != nil { + return session + } + return template +} + type buildSandboxParams struct { namespace string workloadName string @@ -234,23 +245,23 @@ func buildSandboxClaimObject(params *buildSandboxClaimParams) *extensionsv1alpha return sandboxClaim } -func buildSandboxByAgentRuntime(namespace string, name string, ifm *Informers) (*sandboxv1alpha1.Sandbox, *sandboxEntry, error) { +func buildSandboxByAgentRuntime(namespace string, name string, ifm *Informers, routerSelector map[string]string, routerNamespace string, sessionNetworkPolicy *runtimev1alpha1.SandboxNetworkPolicy) (*sandboxv1alpha1.Sandbox, *sandboxEntry, *networkingv1.NetworkPolicy, error) { agentRuntimeKey := namespace + "/" + name // TODO(hzxuzhonghu): make use of typed informer, so we don't need to do type conversion below runtimeObj, exists, _ := ifm.AgentRuntimeInformer.GetStore().GetByKey(agentRuntimeKey) if !exists { - return nil, nil, api.ErrAgentRuntimeNotFound + return nil, nil, nil, api.ErrAgentRuntimeNotFound } unstructuredObj, ok := runtimeObj.(*unstructured.Unstructured) if !ok { klog.Errorf("agent runtime %s type asserting unstructured.Unstructured failed", agentRuntimeKey) - return nil, nil, fmt.Errorf("agent runtime type asserting failed") + return nil, nil, nil, fmt.Errorf("agent runtime type asserting failed") } var agentRuntimeObj runtimev1alpha1.AgentRuntime if err := runtime.DefaultUnstructuredConverter.FromUnstructured(unstructuredObj.Object, &agentRuntimeObj); err != nil { - return nil, nil, fmt.Errorf("failed to convert unstructured to AgentRuntime: %w", err) + return nil, nil, nil, fmt.Errorf("failed to convert unstructured to AgentRuntime: %w", err) } sessionID := uuid.New().String() @@ -291,7 +302,8 @@ func buildSandboxByAgentRuntime(namespace string, name string, ifm *Informers) ( SessionID: sessionID, IdleTimeout: idleTimeout, } - return sandbox, entry, nil + np := buildNetworkPolicy(sandboxName, namespace, effectiveNetworkPolicy(sessionNetworkPolicy, agentRuntimeObj.Spec.Template.NetworkPolicy), routerSelector, routerNamespace) + return sandbox, entry, np, nil } // buildCodeInterpreterEnvVars copies the template env vars and injects the @@ -308,30 +320,30 @@ func buildCodeInterpreterEnvVars(templateEnv []corev1.EnvVar, authMode runtimev1 return envVars } -func buildSandboxByCodeInterpreter(namespace string, codeInterpreterName string, informer *Informers) (*sandboxv1alpha1.Sandbox, *extensionsv1alpha1.SandboxClaim, *sandboxEntry, error) { +func buildSandboxByCodeInterpreter(namespace string, codeInterpreterName string, informer *Informers, routerSelector map[string]string, routerNamespace string, sessionNetworkPolicy *runtimev1alpha1.SandboxNetworkPolicy) (*sandboxv1alpha1.Sandbox, *extensionsv1alpha1.SandboxClaim, *networkingv1.NetworkPolicy, *sandboxEntry, error) { codeInterpreterKey := namespace + "/" + codeInterpreterName // TODO(hzxuzhonghu): make use of typed informer, so we don't need to do type conversion below runtimeObj, exists, err := informer.CodeInterpreterInformer.GetStore().GetByKey(codeInterpreterKey) if err != nil { - return nil, nil, nil, fmt.Errorf("failed to get code interpreter %s from informer cache: %w", codeInterpreterKey, err) + return nil, nil, nil, nil, fmt.Errorf("failed to get code interpreter %s from informer cache: %w", codeInterpreterKey, err) } if !exists { - return nil, nil, nil, api.ErrCodeInterpreterNotFound + return nil, nil, nil, nil, api.ErrCodeInterpreterNotFound } unstructuredObj, ok := runtimeObj.(*unstructured.Unstructured) if !ok { klog.Errorf("code interpreter %s type asserting unstructured.Unstructured failed", codeInterpreterKey) - return nil, nil, nil, fmt.Errorf("code interpreter type asserting failed") + return nil, nil, nil, nil, fmt.Errorf("code interpreter type asserting failed") } var codeInterpreterObj runtimev1alpha1.CodeInterpreter if err := runtime.DefaultUnstructuredConverter.FromUnstructured(unstructuredObj.Object, &codeInterpreterObj); err != nil { - return nil, nil, nil, fmt.Errorf("failed to convert unstructured to CodeInterpreter: %w", err) + return nil, nil, nil, nil, fmt.Errorf("failed to convert unstructured to CodeInterpreter: %w", err) } // Check public key available if authMode is picod if codeInterpreterObj.Spec.AuthMode == runtimev1alpha1.AuthModePicoD && !IsPublicKeyCached() { - return nil, nil, nil, api.ErrPublicKeyMissing + return nil, nil, nil, nil, api.ErrPublicKeyMissing } sessionID := uuid.New().String() @@ -388,7 +400,8 @@ func buildSandboxByCodeInterpreter(namespace string, codeInterpreterName string, simpleSandbox.Spec.Lifecycle.ShutdownTime = &shutdownTime } sandboxEntry.Kind = types.SandboxClaimsKind - return simpleSandbox, sandboxClaim, sandboxEntry, nil + np := buildNetworkPolicy(sandboxName, namespace, effectiveNetworkPolicy(sessionNetworkPolicy, codeInterpreterObj.Spec.Template.NetworkPolicy), routerSelector, routerNamespace) + return simpleSandbox, sandboxClaim, np, sandboxEntry, nil } // Normalize RuntimeClassName: if it's an empty string, set it to nil @@ -428,5 +441,6 @@ func buildSandboxByCodeInterpreter(namespace string, codeInterpreterName string, buildParams.ttl = codeInterpreterObj.Spec.MaxSessionDuration.Duration } sandbox := buildSandboxObject(buildParams) - return sandbox, nil, sandboxEntry, nil + np := buildNetworkPolicy(sandboxName, namespace, codeInterpreterObj.Spec.Template.NetworkPolicy, routerSelector, routerNamespace) + return sandbox, nil, np, sandboxEntry, nil } From bf2393753821740dab88266b93718c8671be6f4b Mon Sep 17 00:00:00 2001 From: Sanchit2662 Date: Tue, 21 Apr 2026 00:11:57 +0530 Subject: [PATCH 2/7] fix: apply session NetworkPolicy override in non-warmpool CodeInterpreter path - Fix session NP override being ignored in direct sandbox path - Add CIDR format validation markers to SandboxEgressRule and SandboxIngressRule - Add example manifest for NetworkPolicy modes (Restricted, Custom) Signed-off-by: Sanchit2662 --- .../code-interpreter-network-policy.yaml | 108 ++++++++++++++++++ ...me.agentcube.volcano.sh_agentruntimes.yaml | 12 +- ...agentcube.volcano.sh_codeinterpreters.yaml | 12 +- .../runtime/v1alpha1/networkpolicy_types.go | 6 + pkg/workloadmanager/workload_builder.go | 2 +- 5 files changed, 135 insertions(+), 5 deletions(-) create mode 100644 example/code-interpreter/code-interpreter-network-policy.yaml diff --git a/example/code-interpreter/code-interpreter-network-policy.yaml b/example/code-interpreter/code-interpreter-network-policy.yaml new file mode 100644 index 00000000..23050641 --- /dev/null +++ b/example/code-interpreter/code-interpreter-network-policy.yaml @@ -0,0 +1,108 @@ +# Example: CodeInterpreter with NetworkPolicy enabled +# +# Three examples are shown below: +# +# 1. Restricted mode - deny-all baseline, DNS and router ingress always allowed +# 2. Restricted mode with extra egress - allow outbound to a specific CIDR +# 3. Custom mode - bring your own raw NetworkPolicy rules +# +# Default mode is "None" (no NetworkPolicy created), so existing setups are unaffected. + +--- +# 1. Restricted mode — simplest secure setup +apiVersion: runtime.agentcube.volcano.sh/v1alpha1 +kind: CodeInterpreter +metadata: + name: interpreter-restricted + namespace: default +spec: + ports: + - pathPrefix: "/" + port: 8080 + protocol: "HTTP" + template: + image: ghcr.io/volcano-sh/picod:latest + imagePullPolicy: IfNotPresent + resources: + limits: + cpu: "500m" + memory: "512Mi" + requests: + cpu: "100m" + memory: "128Mi" + networkPolicy: + mode: Restricted + sessionTimeout: "15m" + maxSessionDuration: "8h" + +--- +# 2. Restricted mode with additional egress — allow outbound to 10.0.0.0/8 on port 443 +apiVersion: runtime.agentcube.volcano.sh/v1alpha1 +kind: CodeInterpreter +metadata: + name: interpreter-restricted-egress + namespace: default +spec: + ports: + - pathPrefix: "/" + port: 8080 + protocol: "HTTP" + template: + image: ghcr.io/volcano-sh/picod:latest + imagePullPolicy: IfNotPresent + resources: + limits: + cpu: "500m" + memory: "512Mi" + requests: + cpu: "100m" + memory: "128Mi" + networkPolicy: + mode: Restricted + allowedEgress: + - cidrs: + - "10.0.0.0/8" + ports: + - protocol: TCP + port: 443 + sessionTimeout: "15m" + maxSessionDuration: "8h" + +--- +# 3. Custom mode — raw NetworkPolicy rules, full control +apiVersion: runtime.agentcube.volcano.sh/v1alpha1 +kind: CodeInterpreter +metadata: + name: interpreter-custom + namespace: default +spec: + ports: + - pathPrefix: "/" + port: 8080 + protocol: "HTTP" + template: + image: ghcr.io/volcano-sh/picod:latest + imagePullPolicy: IfNotPresent + resources: + limits: + cpu: "500m" + memory: "512Mi" + requests: + cpu: "100m" + memory: "128Mi" + networkPolicy: + mode: Custom + custom: + ingress: + - from: + - podSelector: + matchLabels: + app: agentcube-router + egress: + - ports: + - protocol: UDP + port: 53 + - protocol: TCP + port: 53 + sessionTimeout: "15m" + maxSessionDuration: "8h" diff --git a/manifests/charts/base/crds/runtime.agentcube.volcano.sh_agentruntimes.yaml b/manifests/charts/base/crds/runtime.agentcube.volcano.sh_agentruntimes.yaml index 460ec327..54fec750 100644 --- a/manifests/charts/base/crds/runtime.agentcube.volcano.sh_agentruntimes.yaml +++ b/manifests/charts/base/crds/runtime.agentcube.volcano.sh_agentruntimes.yaml @@ -81,10 +81,14 @@ spec: rule for a sandbox pod. properties: cidrs: - description: CIDRs is the list of destination CIDR blocks. + description: |- + CIDRs is the list of destination CIDR blocks. + Each entry must be a valid CIDR notation (e.g. "10.0.0.0/8" or "2001:db8::/32"). items: + pattern: ^(\d{1,3}\.){3}\d{1,3}\/\d{1,2}$|^[0-9a-fA-F:]+\/\d{1,3}$ type: string type: array + x-kubernetes-list-type: atomic ports: description: Ports restricts the rule to these destination ports. An empty list matches all ports. @@ -118,10 +122,14 @@ spec: rule for a sandbox pod. properties: cidrs: - description: CIDRs is the list of source CIDR blocks. + description: |- + CIDRs is the list of source CIDR blocks. + Each entry must be a valid CIDR notation (e.g. "10.0.0.0/8" or "2001:db8::/32"). items: + pattern: ^(\d{1,3}\.){3}\d{1,3}\/\d{1,2}$|^[0-9a-fA-F:]+\/\d{1,3}$ type: string type: array + x-kubernetes-list-type: atomic ports: description: Ports restricts the rule to these destination ports. An empty list matches all ports. diff --git a/manifests/charts/base/crds/runtime.agentcube.volcano.sh_codeinterpreters.yaml b/manifests/charts/base/crds/runtime.agentcube.volcano.sh_codeinterpreters.yaml index 8535dd86..91a5485b 100644 --- a/manifests/charts/base/crds/runtime.agentcube.volcano.sh_codeinterpreters.yaml +++ b/manifests/charts/base/crds/runtime.agentcube.volcano.sh_codeinterpreters.yaml @@ -350,10 +350,14 @@ spec: rule for a sandbox pod. properties: cidrs: - description: CIDRs is the list of destination CIDR blocks. + description: |- + CIDRs is the list of destination CIDR blocks. + Each entry must be a valid CIDR notation (e.g. "10.0.0.0/8" or "2001:db8::/32"). items: + pattern: ^(\d{1,3}\.){3}\d{1,3}\/\d{1,2}$|^[0-9a-fA-F:]+\/\d{1,3}$ type: string type: array + x-kubernetes-list-type: atomic ports: description: Ports restricts the rule to these destination ports. An empty list matches all ports. @@ -387,10 +391,14 @@ spec: rule for a sandbox pod. properties: cidrs: - description: CIDRs is the list of source CIDR blocks. + description: |- + CIDRs is the list of source CIDR blocks. + Each entry must be a valid CIDR notation (e.g. "10.0.0.0/8" or "2001:db8::/32"). items: + pattern: ^(\d{1,3}\.){3}\d{1,3}\/\d{1,2}$|^[0-9a-fA-F:]+\/\d{1,3}$ type: string type: array + x-kubernetes-list-type: atomic ports: description: Ports restricts the rule to these destination ports. An empty list matches all ports. diff --git a/pkg/apis/runtime/v1alpha1/networkpolicy_types.go b/pkg/apis/runtime/v1alpha1/networkpolicy_types.go index ff5dcfb4..fc9150b0 100644 --- a/pkg/apis/runtime/v1alpha1/networkpolicy_types.go +++ b/pkg/apis/runtime/v1alpha1/networkpolicy_types.go @@ -68,7 +68,10 @@ type SandboxNetworkPolicy struct { // SandboxEgressRule describes an egress allow rule for a sandbox pod. type SandboxEgressRule struct { // CIDRs is the list of destination CIDR blocks. + // Each entry must be a valid CIDR notation (e.g. "10.0.0.0/8" or "2001:db8::/32"). // +optional + // +listType=atomic + // +kubebuilder:validation:items:Pattern=`^(\d{1,3}\.){3}\d{1,3}\/\d{1,2}$|^[0-9a-fA-F:]+\/\d{1,3}$` CIDRs []string `json:"cidrs,omitempty"` // Ports restricts the rule to these destination ports. An empty list matches all ports. // +optional @@ -78,7 +81,10 @@ type SandboxEgressRule struct { // SandboxIngressRule describes an ingress allow rule for a sandbox pod. type SandboxIngressRule struct { // CIDRs is the list of source CIDR blocks. + // Each entry must be a valid CIDR notation (e.g. "10.0.0.0/8" or "2001:db8::/32"). // +optional + // +listType=atomic + // +kubebuilder:validation:items:Pattern=`^(\d{1,3}\.){3}\d{1,3}\/\d{1,2}$|^[0-9a-fA-F:]+\/\d{1,3}$` CIDRs []string `json:"cidrs,omitempty"` // Ports restricts the rule to these destination ports. An empty list matches all ports. // +optional diff --git a/pkg/workloadmanager/workload_builder.go b/pkg/workloadmanager/workload_builder.go index 4d6dc73d..9a7db60f 100644 --- a/pkg/workloadmanager/workload_builder.go +++ b/pkg/workloadmanager/workload_builder.go @@ -441,6 +441,6 @@ func buildSandboxByCodeInterpreter(namespace string, codeInterpreterName string, buildParams.ttl = codeInterpreterObj.Spec.MaxSessionDuration.Duration } sandbox := buildSandboxObject(buildParams) - np := buildNetworkPolicy(sandboxName, namespace, codeInterpreterObj.Spec.Template.NetworkPolicy, routerSelector, routerNamespace) + np := buildNetworkPolicy(sandboxName, namespace, effectiveNetworkPolicy(sessionNetworkPolicy, codeInterpreterObj.Spec.Template.NetworkPolicy), routerSelector, routerNamespace) return sandbox, nil, np, sandboxEntry, nil } From 2498ec27ece87d5a690248243e3cfcd34633c2f9 Mon Sep 17 00:00:00 2001 From: Sanchit2662 Date: Tue, 21 Apr 2026 11:02:27 +0530 Subject: [PATCH 3/7] fix: address reviewer feedback on NetworkPolicy implementation - Add OwnerReference on NetworkPolicy pointing to Sandbox/SandboxClaim so k8s GC cascades deletion when owner is removed out-of-band - Add CEL validation: custom field must be set when mode is Custom - Add TODO comment on warm-pool NP selector gap pending agent-sandbox fix - Add GC lifecycle tests verifying NP is deleted alongside sandbox - Extend createSandboxClaim to return UID for OwnerReference wiring Signed-off-by: Sanchit2662 --- ...me.agentcube.volcano.sh_agentruntimes.yaml | 4 ++ ...agentcube.volcano.sh_codeinterpreters.yaml | 4 ++ .../runtime/v1alpha1/networkpolicy_types.go | 1 + .../garbage_collection_test.go | 52 +++++++++++++++++++ pkg/workloadmanager/handlers.go | 28 ++++++++-- pkg/workloadmanager/handlers_test.go | 7 +-- pkg/workloadmanager/k8s_client.go | 19 ++++--- pkg/workloadmanager/workload_builder.go | 4 ++ 8 files changed, 105 insertions(+), 14 deletions(-) diff --git a/manifests/charts/base/crds/runtime.agentcube.volcano.sh_agentruntimes.yaml b/manifests/charts/base/crds/runtime.agentcube.volcano.sh_agentruntimes.yaml index 54fec750..c49ab754 100644 --- a/manifests/charts/base/crds/runtime.agentcube.volcano.sh_agentruntimes.yaml +++ b/manifests/charts/base/crds/runtime.agentcube.volcano.sh_agentruntimes.yaml @@ -551,6 +551,10 @@ spec: - Custom type: string type: object + x-kubernetes-validations: + - message: custom must be set when mode is Custom + rule: self.mode != 'Custom' || (has(self.custom) && self.custom + != null) spec: description: Spec is the Pod's spec properties: diff --git a/manifests/charts/base/crds/runtime.agentcube.volcano.sh_codeinterpreters.yaml b/manifests/charts/base/crds/runtime.agentcube.volcano.sh_codeinterpreters.yaml index 91a5485b..2ada2f27 100644 --- a/manifests/charts/base/crds/runtime.agentcube.volcano.sh_codeinterpreters.yaml +++ b/manifests/charts/base/crds/runtime.agentcube.volcano.sh_codeinterpreters.yaml @@ -820,6 +820,10 @@ spec: - Custom type: string type: object + x-kubernetes-validations: + - message: custom must be set when mode is Custom + rule: self.mode != 'Custom' || (has(self.custom) && self.custom + != null) resources: description: |- Compute Resources required by this container. diff --git a/pkg/apis/runtime/v1alpha1/networkpolicy_types.go b/pkg/apis/runtime/v1alpha1/networkpolicy_types.go index fc9150b0..9fbed948 100644 --- a/pkg/apis/runtime/v1alpha1/networkpolicy_types.go +++ b/pkg/apis/runtime/v1alpha1/networkpolicy_types.go @@ -37,6 +37,7 @@ const ( ) // SandboxNetworkPolicy defines the network isolation policy applied to each sandbox pod. +// +kubebuilder:validation:XValidation:rule="self.mode != 'Custom' || (has(self.custom) && self.custom != null)",message="custom must be set when mode is Custom" type SandboxNetworkPolicy struct { // Mode controls the network policy enforcement level. // "None" (default): no NetworkPolicy is created. diff --git a/pkg/workloadmanager/garbage_collection_test.go b/pkg/workloadmanager/garbage_collection_test.go index 3e6cf7a5..21004660 100644 --- a/pkg/workloadmanager/garbage_collection_test.go +++ b/pkg/workloadmanager/garbage_collection_test.go @@ -23,6 +23,9 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + networkingv1 "k8s.io/api/networking/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" dynamicfake "k8s.io/client-go/dynamic/fake" @@ -275,3 +278,52 @@ func TestGarbageCollector_once_noSandboxes(t *testing.T) { gc.once() assert.Empty(t, fs.deleted) } + +// TestGarbageCollector_NP_cleanup verifies that the GC deletes the NetworkPolicy +// associated with a sandbox, including when the sandbox itself is already gone. +func TestGarbageCollector_NP_cleanup(t *testing.T) { + tests := []struct { + name string + kind string + sbName string + session string + }{ + {name: "sandbox kind deletes NP", kind: types.SandboxKind, sbName: "sb-np", session: "sess-np"}, + {name: "sandbox claim kind deletes NP", kind: types.SandboxClaimsKind, sbName: "sc-np", session: "sess-np-claim"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fs := &gcFakeStore{ + expired: []*types.SandboxInfo{{ + SessionID: tt.session, + Kind: tt.kind, + Name: tt.sbName, + SandboxNamespace: "default", + }}, + } + gc := newTestGC(fs) + + // Pre-create a NetworkPolicy with the same name as the sandbox + // so we can verify the GC deletes it. + np := &networkingv1.NetworkPolicy{} + np.Name = tt.sbName + np.Namespace = "default" + _, err := gc.k8sClient.clientset.NetworkingV1().NetworkPolicies("default").Create( + context.Background(), np, metav1.CreateOptions{}, + ) + require.NoError(t, err) + + gc.once() + + // Session must be removed from store + assert.Contains(t, fs.deleted, tt.session) + + // NetworkPolicy must be gone + _, err = gc.k8sClient.clientset.NetworkingV1().NetworkPolicies("default").Get( + context.Background(), tt.sbName, metav1.GetOptions{}, + ) + assert.True(t, apierrors.IsNotFound(err), "NetworkPolicy should have been deleted by GC") + }) + } +} diff --git a/pkg/workloadmanager/handlers.go b/pkg/workloadmanager/handlers.go index 1d3b1fed..a2200455 100644 --- a/pkg/workloadmanager/handlers.go +++ b/pkg/workloadmanager/handlers.go @@ -26,7 +26,10 @@ import ( "github.com/gin-gonic/gin" networkingv1 "k8s.io/api/networking/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + k8stypes "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/dynamic" + "k8s.io/utils/ptr" "k8s.io/klog/v2" sandboxv1alpha1 "sigs.k8s.io/agent-sandbox/api/v1alpha1" "sigs.k8s.io/agent-sandbox/controllers" @@ -151,15 +154,32 @@ func (s *Server) createSandbox(ctx context.Context, dynamicClient dynamic.Interf return nil, err } + var ownerUID k8stypes.UID + var ownerAPIVersion, ownerKind, ownerName string if sandboxClaim != nil { - if err := createSandboxClaim(ctx, dynamicClient, sandboxClaim); err != nil { - err = api.NewInternalError(fmt.Errorf("create sandbox claim %s/%s failed: %v", sandboxClaim.Namespace, sandboxClaim.Name, err)) - return nil, err + uid, err := createSandboxClaim(ctx, dynamicClient, sandboxClaim) + if err != nil { + return nil, api.NewInternalError(fmt.Errorf("create sandbox claim %s/%s failed: %v", sandboxClaim.Namespace, sandboxClaim.Name, err)) } + ownerUID, ownerAPIVersion, ownerKind, ownerName = uid, "extensions.agents.x-k8s.io/v1alpha1", "SandboxClaim", sandboxClaim.Name } else { - if _, err := createSandbox(ctx, dynamicClient, sandbox); err != nil { + info, err := createSandbox(ctx, dynamicClient, sandbox) + if err != nil { return nil, api.NewInternalError(fmt.Errorf("failed to create sandbox: %w", err)) } + ownerUID, ownerAPIVersion, ownerKind, ownerName = info.UID, "agents.x-k8s.io/v1alpha1", "Sandbox", info.Name + } + // Attach OwnerReference so k8s GC can cascade-delete the NP if the owner + // is deleted out-of-band (e.g. kubectl delete sandbox). This is a safety net + // on top of the explicit cleanup in the delete handler and GC loop. + if networkPolicy != nil && ownerUID != "" { + networkPolicy.OwnerReferences = []metav1.OwnerReference{{ + APIVersion: ownerAPIVersion, + Kind: ownerKind, + Name: ownerName, + UID: ownerUID, + BlockOwnerDeletion: ptr.To(true), + }} } // Register rollback IMMEDIATELY after the sandbox/claim exists, before any diff --git a/pkg/workloadmanager/handlers_test.go b/pkg/workloadmanager/handlers_test.go index a49aef63..fa5edd78 100644 --- a/pkg/workloadmanager/handlers_test.go +++ b/pkg/workloadmanager/handlers_test.go @@ -38,6 +38,7 @@ import ( networkingv1 "k8s.io/api/networking/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + k8stypes "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/dynamic" sandboxv1alpha1 "sigs.k8s.io/agent-sandbox/api/v1alpha1" "sigs.k8s.io/agent-sandbox/controllers" @@ -209,12 +210,12 @@ func TestServerCreateSandbox(t *testing.T) { return &SandboxInfo{Name: sandbox.Name, Namespace: sandbox.Namespace}, nil }) - patches.ApplyFunc(createSandboxClaim, func(_ context.Context, _ dynamic.Interface, _ *extensionsv1alpha1.SandboxClaim) error { + patches.ApplyFunc(createSandboxClaim, func(_ context.Context, _ dynamic.Interface, _ *extensionsv1alpha1.SandboxClaim) (k8stypes.UID, error) { claimCalls++ if tt.createClaimErr != nil { - return tt.createClaimErr + return "", tt.createClaimErr } - return nil + return "fake-claim-uid", nil }) patches.ApplyFunc(deleteSandbox, func(_ context.Context, _ dynamic.Interface, _, _ string) error { diff --git a/pkg/workloadmanager/k8s_client.go b/pkg/workloadmanager/k8s_client.go index 6aa3483c..cc8e58e7 100644 --- a/pkg/workloadmanager/k8s_client.go +++ b/pkg/workloadmanager/k8s_client.go @@ -31,6 +31,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" + k8stypes "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/dynamic" "k8s.io/client-go/dynamic/dynamicinformer" "k8s.io/client-go/informers" @@ -146,6 +147,7 @@ func NewK8sClient() (*K8sClient, error) { type SandboxInfo struct { Name string Namespace string + UID k8stypes.UID } // UserK8sClient creates a temporary Kubernetes client using user's token @@ -221,30 +223,32 @@ func createSandbox(ctx context.Context, client dynamic.Interface, sandbox *sandb return &SandboxInfo{ Name: created.GetName(), Namespace: created.GetNamespace(), + UID: created.GetUID(), }, nil } -// createSandboxClaim creates a SandboxClaim using the provided dynamic client -func createSandboxClaim(ctx context.Context, client dynamic.Interface, sandboxClaim *extensionsv1alpha1.SandboxClaim) error { +// createSandboxClaim creates a SandboxClaim using the provided dynamic client. +// Returns the UID of the created SandboxClaim so callers can set OwnerReferences. +func createSandboxClaim(ctx context.Context, client dynamic.Interface, sandboxClaim *extensionsv1alpha1.SandboxClaim) (k8stypes.UID, error) { // Convert to unstructured for dynamic client unstructuredObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(sandboxClaim) if err != nil { - return fmt.Errorf("failed to convert sandbox claim to unstructured: %w", err) + return "", fmt.Errorf("failed to convert sandbox claim to unstructured: %w", err) } unstructuredSandbox := &unstructured.Unstructured{Object: unstructuredObj} // Create SandboxClaim - _, err = client.Resource(SandboxClaimGVR).Namespace(sandboxClaim.Namespace).Create( + created, err := client.Resource(SandboxClaimGVR).Namespace(sandboxClaim.Namespace).Create( ctx, unstructuredSandbox, metav1.CreateOptions{}, ) if err != nil { - return fmt.Errorf("failed to create sandbox claim: %w", err) + return "", fmt.Errorf("failed to create sandbox claim: %w", err) } - return nil + return created.GetUID(), nil } // deleteSandbox deletes a Sandbox using the provided dynamic client @@ -277,7 +281,8 @@ func deleteSandboxClaim(ctx context.Context, client dynamic.Interface, namespace // CreateSandboxClaim creates a new SandboxClaim using user's permissions func (u *UserK8sClient) CreateSandboxClaim(ctx context.Context, sandboxClaim *extensionsv1alpha1.SandboxClaim) error { - return createSandboxClaim(ctx, u.dynamicClient, sandboxClaim) + _, err := createSandboxClaim(ctx, u.dynamicClient, sandboxClaim) + return err } // CreateSandbox creates a new Sandbox using user's permissions diff --git a/pkg/workloadmanager/workload_builder.go b/pkg/workloadmanager/workload_builder.go index 9a7db60f..c7c35011 100644 --- a/pkg/workloadmanager/workload_builder.go +++ b/pkg/workloadmanager/workload_builder.go @@ -401,6 +401,10 @@ func buildSandboxByCodeInterpreter(namespace string, codeInterpreterName string, } sandboxEntry.Kind = types.SandboxClaimsKind np := buildNetworkPolicy(sandboxName, namespace, effectiveNetworkPolicy(sessionNetworkPolicy, codeInterpreterObj.Spec.Template.NetworkPolicy), routerSelector, routerNamespace) + // TODO: warm-pool pods do not currently carry SandboxNameLabelKey, so the + // NP PodSelector won't match them and enforcement is a no-op for this path. + // Fix requires agent-sandbox to propagate claim labels onto the bound pod, + // or a post-ready pod patch. Tracked in issue #216. return simpleSandbox, sandboxClaim, np, sandboxEntry, nil } From 9963991a1f08bb0dbdb6430f9f6d446c1a8e8c2b Mon Sep 17 00:00:00 2001 From: Sanchit2662 Date: Tue, 21 Apr 2026 11:08:25 +0530 Subject: [PATCH 4/7] fix: reduce cyclomatic complexity of createSandbox below lint threshold Extract sandbox/claim creation and NetworkPolicy OwnerReference wiring into createWorkload helper to bring createSandbox complexity from 16 to under the gocyclo limit of 15 Also address Copilot review comments: - Treat AlreadyExists as error in createNetworkPolicy instead of silently reusing a potentially stale or mismatched policy - Validate per-session NetworkPolicy override in CreateSandboxRequest.Validate: reject unknown modes and require custom field when mode is Custom - Fix import ordering to be goimports-compliant across handlers.go, k8s_client.go, sandbox.go and main.go - Warn at startup when --router-selector parses to an empty map Signed-off-by: Sanchit2662 --- cmd/workload-manager/main.go | 7 +++- pkg/common/types/sandbox.go | 26 +++++++++++- pkg/workloadmanager/handlers.go | 68 ++++++++++++++++++------------- pkg/workloadmanager/k8s_client.go | 5 ++- 4 files changed, 74 insertions(+), 32 deletions(-) diff --git a/cmd/workload-manager/main.go b/cmd/workload-manager/main.go index 352933fd..d7f7bab8 100644 --- a/cmd/workload-manager/main.go +++ b/cmd/workload-manager/main.go @@ -103,6 +103,11 @@ func main() { os.Exit(1) } + sel := parseSelector(*routerSelector) + if len(sel) == 0 { + klog.Warningf("--router-selector %q parsed to an empty selector; falling back to default %v", *routerSelector, workloadmanager.DefaultRouterSelector) + } + // Create API server configuration config := &workloadmanager.Config{ Port: *port, @@ -111,7 +116,7 @@ func main() { TLSCert: *tlsCert, TLSKey: *tlsKey, EnableAuth: *enableAuth, - RouterSelector: parseSelector(*routerSelector), + RouterSelector: sel, RouterNamespace: *routerNamespace, } diff --git a/pkg/common/types/sandbox.go b/pkg/common/types/sandbox.go index 7286df0f..7d5e295b 100644 --- a/pkg/common/types/sandbox.go +++ b/pkg/common/types/sandbox.go @@ -20,8 +20,8 @@ import ( "fmt" "time" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtimev1alpha1 "github.com/volcano-sh/agentcube/pkg/apis/runtime/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) type SandboxInfo struct { @@ -81,5 +81,29 @@ func (car *CreateSandboxRequest) Validate() error { if car.Name == "" { return fmt.Errorf("name is required") } + if err := validateNetworkPolicyOverride(car.NetworkPolicy); err != nil { + return fmt.Errorf("invalid networkPolicy: %w", err) + } + return nil +} + +// validateNetworkPolicyOverride validates a per-session NetworkPolicy override from +// CreateSandboxRequest. Unlike the CRD field (which has kubebuilder markers), this +// comes in as raw JSON and bypasses API-server admission, so we validate it here. +func validateNetworkPolicyOverride(np *runtimev1alpha1.SandboxNetworkPolicy) error { + if np == nil { + return nil + } + switch np.Mode { + case runtimev1alpha1.NetworkPolicyModeNone, + runtimev1alpha1.NetworkPolicyModeRestricted, + runtimev1alpha1.NetworkPolicyModeCustom, + "": // empty is treated as None + default: + return fmt.Errorf("unknown mode %q: must be one of None, Restricted, Custom", np.Mode) + } + if np.Mode == runtimev1alpha1.NetworkPolicyModeCustom && np.Custom == nil { + return fmt.Errorf("custom must be set when mode is Custom") + } return nil } diff --git a/pkg/workloadmanager/handlers.go b/pkg/workloadmanager/handlers.go index a2200455..ff52d767 100644 --- a/pkg/workloadmanager/handlers.go +++ b/pkg/workloadmanager/handlers.go @@ -29,8 +29,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" k8stypes "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/dynamic" - "k8s.io/utils/ptr" "k8s.io/klog/v2" + "k8s.io/utils/ptr" sandboxv1alpha1 "sigs.k8s.io/agent-sandbox/api/v1alpha1" "sigs.k8s.io/agent-sandbox/controllers" extensionsv1alpha1 "sigs.k8s.io/agent-sandbox/extensions/api/v1alpha1" @@ -145,41 +145,51 @@ func (s *Server) handleSandboxCreate(c *gin.Context, kind string) { respondJSON(c, http.StatusOK, response) } +// createWorkload creates the Sandbox or SandboxClaim and, when a NetworkPolicy is provided, +// wires the created object's UID into the NP's OwnerReferences so k8s GC can +// cascade-delete the NP if the owner is removed out-of-band. +func createWorkload(ctx context.Context, client dynamic.Interface, sandbox *sandboxv1alpha1.Sandbox, sandboxClaim *extensionsv1alpha1.SandboxClaim, networkPolicy *networkingv1.NetworkPolicy) error { + if sandboxClaim != nil { + uid, err := createSandboxClaim(ctx, client, sandboxClaim) + if err != nil { + return api.NewInternalError(fmt.Errorf("create sandbox claim %s/%s failed: %v", sandboxClaim.Namespace, sandboxClaim.Name, err)) + } + setNetworkPolicyOwner(networkPolicy, "extensions.agents.x-k8s.io/v1alpha1", "SandboxClaim", sandboxClaim.Name, uid) + return nil + } + info, err := createSandbox(ctx, client, sandbox) + if err != nil { + return api.NewInternalError(fmt.Errorf("failed to create sandbox: %w", err)) + } + setNetworkPolicyOwner(networkPolicy, "agents.x-k8s.io/v1alpha1", "Sandbox", info.Name, info.UID) + return nil +} + +// setNetworkPolicyOwner attaches an OwnerReference to np pointing at the given owner. +// No-op when np is nil or uid is empty. +func setNetworkPolicyOwner(np *networkingv1.NetworkPolicy, apiVersion, kind, name string, uid k8stypes.UID) { + if np == nil || uid == "" { + return + } + np.OwnerReferences = []metav1.OwnerReference{{ + APIVersion: apiVersion, + Kind: kind, + Name: name, + UID: uid, + BlockOwnerDeletion: ptr.To(true), + }} +} + // createSandbox performs sandbox creation and returns the response payload or an error with an HTTP status code. func (s *Server) createSandbox(ctx context.Context, dynamicClient dynamic.Interface, sandbox *sandboxv1alpha1.Sandbox, sandboxClaim *extensionsv1alpha1.SandboxClaim, networkPolicy *networkingv1.NetworkPolicy, sandboxEntry *sandboxEntry, resultChan <-chan SandboxStatusUpdate) (*types.CreateSandboxResponse, error) { // Store placeholder before creating, make sandbox/sandboxClaim GarbageCollection possible sandboxStorePlaceHolder := buildSandboxPlaceHolder(sandbox, sandboxEntry) if err := s.storeClient.StoreSandbox(ctx, sandboxStorePlaceHolder); err != nil { - err = api.NewInternalError(fmt.Errorf("store sandbox placeholder failed: %v", err)) - return nil, err + return nil, api.NewInternalError(fmt.Errorf("store sandbox placeholder failed: %v", err)) } - var ownerUID k8stypes.UID - var ownerAPIVersion, ownerKind, ownerName string - if sandboxClaim != nil { - uid, err := createSandboxClaim(ctx, dynamicClient, sandboxClaim) - if err != nil { - return nil, api.NewInternalError(fmt.Errorf("create sandbox claim %s/%s failed: %v", sandboxClaim.Namespace, sandboxClaim.Name, err)) - } - ownerUID, ownerAPIVersion, ownerKind, ownerName = uid, "extensions.agents.x-k8s.io/v1alpha1", "SandboxClaim", sandboxClaim.Name - } else { - info, err := createSandbox(ctx, dynamicClient, sandbox) - if err != nil { - return nil, api.NewInternalError(fmt.Errorf("failed to create sandbox: %w", err)) - } - ownerUID, ownerAPIVersion, ownerKind, ownerName = info.UID, "agents.x-k8s.io/v1alpha1", "Sandbox", info.Name - } - // Attach OwnerReference so k8s GC can cascade-delete the NP if the owner - // is deleted out-of-band (e.g. kubectl delete sandbox). This is a safety net - // on top of the explicit cleanup in the delete handler and GC loop. - if networkPolicy != nil && ownerUID != "" { - networkPolicy.OwnerReferences = []metav1.OwnerReference{{ - APIVersion: ownerAPIVersion, - Kind: ownerKind, - Name: ownerName, - UID: ownerUID, - BlockOwnerDeletion: ptr.To(true), - }} + if err := createWorkload(ctx, dynamicClient, sandbox, sandboxClaim, networkPolicy); err != nil { + return nil, err } // Register rollback IMMEDIATELY after the sandbox/claim exists, before any diff --git a/pkg/workloadmanager/k8s_client.go b/pkg/workloadmanager/k8s_client.go index cc8e58e7..631e71e4 100644 --- a/pkg/workloadmanager/k8s_client.go +++ b/pkg/workloadmanager/k8s_client.go @@ -391,9 +391,12 @@ func (c *K8sClient) GetSandboxInformer() cache.SharedInformer { } // createNetworkPolicy creates a NetworkPolicy using the workloadmanager's own client. +// AlreadyExists is treated as an error: sandbox names embed random suffixes so a +// collision means something unexpected is already occupying the name, and silently +// reusing it could leave the sandbox under a stale or incorrect policy. func createNetworkPolicy(ctx context.Context, clientset kubernetes.Interface, np *networkingv1.NetworkPolicy) error { _, err := clientset.NetworkingV1().NetworkPolicies(np.Namespace).Create(ctx, np, metav1.CreateOptions{}) - if err != nil && !apierrors.IsAlreadyExists(err) { + if err != nil { return fmt.Errorf("failed to create network policy %s/%s: %w", np.Namespace, np.Name, err) } return nil From 2551dea3cfddeae7921907209d2a5b840ea8d6f3 Mon Sep 17 00:00:00 2001 From: Sanchit2662 Date: Fri, 24 Apr 2026 11:04:17 +0530 Subject: [PATCH 5/7] fix: address reviewer feedback on NetworkPolicy API simplification - Remove Mode enum (None/Restricted/Custom); nil policy means no enforcement, non-nil enforces deny-all with router ingress and DNS - Remove AllowDNS toggle; DNS egress is always included when a policy is set - Rename AllowedEgress/AllowedIngress to Egress/Ingress; update JSON tags to egress/ingress - Merge SandboxEgressRule and SandboxIngressRule into single SandboxNetworkPolicyRule type - Block Custom mode in per-session overrides to prevent RBAC privilege escalation via the workloadmanager elevated SA - Add klog.Fatalf guard when routerNamespace is empty to prevent silent same-namespace-only router ingress semantics - Remove dead IsNotFound branches in handleDeleteSandbox; helpers already swallow NotFound and return nil - Fix incorrect Ports comment copied from SandboxEgressRule into SandboxIngressRule Signed-off-by: Sanchit2662 --- .../code-interpreter-network-policy.yaml | 63 ++------- .../runtime/v1alpha1/networkpolicy_types.go | 80 ++---------- .../runtime/v1alpha1/zz_generated.deepcopy.go | 87 ++----------- pkg/common/types/sandbox.go | 16 +-- pkg/workloadmanager/handlers.go | 29 ++--- pkg/workloadmanager/handlers_test.go | 7 +- pkg/workloadmanager/networkpolicy_builder.go | 68 +++++----- .../networkpolicy_builder_test.go | 121 +++++------------- 8 files changed, 109 insertions(+), 362 deletions(-) diff --git a/example/code-interpreter/code-interpreter-network-policy.yaml b/example/code-interpreter/code-interpreter-network-policy.yaml index 23050641..56f926d8 100644 --- a/example/code-interpreter/code-interpreter-network-policy.yaml +++ b/example/code-interpreter/code-interpreter-network-policy.yaml @@ -1,15 +1,15 @@ # Example: CodeInterpreter with NetworkPolicy enabled # -# Three examples are shown below: +# Set networkPolicy to a non-nil value to enforce isolation. A deny-all baseline +# is created with automatic allow rules for router ingress and DNS egress. +# Leave networkPolicy unset (nil) to disable enforcement entirely. # -# 1. Restricted mode - deny-all baseline, DNS and router ingress always allowed -# 2. Restricted mode with extra egress - allow outbound to a specific CIDR -# 3. Custom mode - bring your own raw NetworkPolicy rules -# -# Default mode is "None" (no NetworkPolicy created), so existing setups are unaffected. +# Two examples are shown: +# 1. Default enforcement — router ingress and DNS egress only +# 2. Extra egress — allow outbound to a specific CIDR/port --- -# 1. Restricted mode — simplest secure setup +# 1. Default enforcement — simplest secure setup apiVersion: runtime.agentcube.volcano.sh/v1alpha1 kind: CodeInterpreter metadata: @@ -30,17 +30,16 @@ spec: requests: cpu: "100m" memory: "128Mi" - networkPolicy: - mode: Restricted + networkPolicy: {} sessionTimeout: "15m" maxSessionDuration: "8h" --- -# 2. Restricted mode with additional egress — allow outbound to 10.0.0.0/8 on port 443 +# 2. Extra egress — allow outbound to 10.0.0.0/8 on port 443 apiVersion: runtime.agentcube.volcano.sh/v1alpha1 kind: CodeInterpreter metadata: - name: interpreter-restricted-egress + name: interpreter-extra-egress namespace: default spec: ports: @@ -58,8 +57,7 @@ spec: cpu: "100m" memory: "128Mi" networkPolicy: - mode: Restricted - allowedEgress: + egress: - cidrs: - "10.0.0.0/8" ports: @@ -67,42 +65,3 @@ spec: port: 443 sessionTimeout: "15m" maxSessionDuration: "8h" - ---- -# 3. Custom mode — raw NetworkPolicy rules, full control -apiVersion: runtime.agentcube.volcano.sh/v1alpha1 -kind: CodeInterpreter -metadata: - name: interpreter-custom - namespace: default -spec: - ports: - - pathPrefix: "/" - port: 8080 - protocol: "HTTP" - template: - image: ghcr.io/volcano-sh/picod:latest - imagePullPolicy: IfNotPresent - resources: - limits: - cpu: "500m" - memory: "512Mi" - requests: - cpu: "100m" - memory: "128Mi" - networkPolicy: - mode: Custom - custom: - ingress: - - from: - - podSelector: - matchLabels: - app: agentcube-router - egress: - - ports: - - protocol: UDP - port: 53 - - protocol: TCP - port: 53 - sessionTimeout: "15m" - maxSessionDuration: "8h" diff --git a/pkg/apis/runtime/v1alpha1/networkpolicy_types.go b/pkg/apis/runtime/v1alpha1/networkpolicy_types.go index 9fbed948..f3be274d 100644 --- a/pkg/apis/runtime/v1alpha1/networkpolicy_types.go +++ b/pkg/apis/runtime/v1alpha1/networkpolicy_types.go @@ -18,76 +18,33 @@ package v1alpha1 import ( corev1 "k8s.io/api/core/v1" - networkingv1 "k8s.io/api/networking/v1" "k8s.io/apimachinery/pkg/util/intstr" ) -// NetworkPolicyMode defines the network policy enforcement level for a sandbox. -// +kubebuilder:validation:Enum=None;Restricted;Custom -type NetworkPolicyMode string - -const ( - // NetworkPolicyModeNone creates no NetworkPolicy (default, backward-compatible). - NetworkPolicyModeNone NetworkPolicyMode = "None" - // NetworkPolicyModeRestricted applies a deny-all baseline with explicit allow rules - // for DNS egress and router ingress. - NetworkPolicyModeRestricted NetworkPolicyMode = "Restricted" - // NetworkPolicyModeCustom uses the raw rules from the Custom field. - NetworkPolicyModeCustom NetworkPolicyMode = "Custom" -) - // SandboxNetworkPolicy defines the network isolation policy applied to each sandbox pod. -// +kubebuilder:validation:XValidation:rule="self.mode != 'Custom' || (has(self.custom) && self.custom != null)",message="custom must be set when mode is Custom" +// When set (non-nil), a deny-all NetworkPolicy is created with explicit allow rules for +// router ingress and DNS egress. Additional rules can be added via Ingress and +// Egress. Leave this field unset (nil) to disable network policy enforcement. type SandboxNetworkPolicy struct { - // Mode controls the network policy enforcement level. - // "None" (default): no NetworkPolicy is created. - // "Restricted": deny-all baseline; DNS egress and router ingress are always allowed. - // "Custom": use the raw Ingress/Egress rules in the Custom field. - // +kubebuilder:default=None + // Egress lists additional egress allow rules beyond the default DNS rule. // +optional - Mode NetworkPolicyMode `json:"mode,omitempty"` + Egress []SandboxNetworkPolicyRule `json:"egress,omitempty"` - // AllowedEgress lists additional egress allow rules applied when Mode=Restricted. + // Ingress lists additional ingress allow rules. + // Router ingress is always permitted regardless of this field. // +optional - AllowedEgress []SandboxEgressRule `json:"allowedEgress,omitempty"` - - // AllowDNS controls whether a DNS egress rule (port 53 UDP/TCP) is added when - // Mode=Restricted. Defaults to true. - // +optional - AllowDNS *bool `json:"allowDNS,omitempty"` - - // AllowedIngress lists additional ingress allow rules when Mode=Restricted. - // Router ingress is always permitted in Restricted mode regardless of this field. - // +optional - AllowedIngress []SandboxIngressRule `json:"allowedIngress,omitempty"` - - // Custom provides raw NetworkPolicy ingress/egress rules, used only when Mode=Custom. - // +optional - Custom *SandboxNetworkPolicyCustomRules `json:"custom,omitempty"` + Ingress []SandboxNetworkPolicyRule `json:"ingress,omitempty"` } -// SandboxEgressRule describes an egress allow rule for a sandbox pod. -type SandboxEgressRule struct { - // CIDRs is the list of destination CIDR blocks. - // Each entry must be a valid CIDR notation (e.g. "10.0.0.0/8" or "2001:db8::/32"). +// SandboxNetworkPolicyRule describes a single ingress or egress allow rule for a sandbox pod. +type SandboxNetworkPolicyRule struct { + // CIDRs is the list of CIDR blocks for this rule (source for ingress, destination for egress). + // Each entry must be valid CIDR notation (e.g. "10.0.0.0/8" or "2001:db8::/32"). // +optional // +listType=atomic // +kubebuilder:validation:items:Pattern=`^(\d{1,3}\.){3}\d{1,3}\/\d{1,2}$|^[0-9a-fA-F:]+\/\d{1,3}$` CIDRs []string `json:"cidrs,omitempty"` - // Ports restricts the rule to these destination ports. An empty list matches all ports. - // +optional - Ports []SandboxNetworkPolicyPort `json:"ports,omitempty"` -} - -// SandboxIngressRule describes an ingress allow rule for a sandbox pod. -type SandboxIngressRule struct { - // CIDRs is the list of source CIDR blocks. - // Each entry must be a valid CIDR notation (e.g. "10.0.0.0/8" or "2001:db8::/32"). - // +optional - // +listType=atomic - // +kubebuilder:validation:items:Pattern=`^(\d{1,3}\.){3}\d{1,3}\/\d{1,2}$|^[0-9a-fA-F:]+\/\d{1,3}$` - CIDRs []string `json:"cidrs,omitempty"` - // Ports restricts the rule to these destination ports. An empty list matches all ports. + // Ports restricts the rule to these ports. An empty list matches all ports. // +optional Ports []SandboxNetworkPolicyPort `json:"ports,omitempty"` } @@ -100,14 +57,3 @@ type SandboxNetworkPolicyPort struct { // Port is the destination port number or named port. Port intstr.IntOrString `json:"port"` } - -// SandboxNetworkPolicyCustomRules holds raw Kubernetes NetworkPolicy ingress/egress rules. -// Used only when Mode=Custom. -type SandboxNetworkPolicyCustomRules struct { - // Ingress is the list of ingress rules. - // +optional - Ingress []networkingv1.NetworkPolicyIngressRule `json:"ingress,omitempty"` - // Egress is the list of egress rules. - // +optional - Egress []networkingv1.NetworkPolicyEgressRule `json:"egress,omitempty"` -} diff --git a/pkg/apis/runtime/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/runtime/v1alpha1/zz_generated.deepcopy.go index 7f03a29d..90763098 100644 --- a/pkg/apis/runtime/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/runtime/v1alpha1/zz_generated.deepcopy.go @@ -22,7 +22,6 @@ package v1alpha1 import ( corev1 "k8s.io/api/core/v1" - networkingv1 "k8s.io/api/networking/v1" "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -327,7 +326,7 @@ func (in *CodeInterpreterStatus) DeepCopy() *CodeInterpreterStatus { } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *SandboxEgressRule) DeepCopyInto(out *SandboxEgressRule) { +func (in *SandboxNetworkPolicyRule) DeepCopyInto(out *SandboxNetworkPolicyRule) { *out = *in if in.CIDRs != nil { in, out := &in.CIDRs, &out.CIDRs @@ -343,39 +342,12 @@ func (in *SandboxEgressRule) DeepCopyInto(out *SandboxEgressRule) { } } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SandboxEgressRule. -func (in *SandboxEgressRule) DeepCopy() *SandboxEgressRule { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SandboxNetworkPolicyRule. +func (in *SandboxNetworkPolicyRule) DeepCopy() *SandboxNetworkPolicyRule { if in == nil { return nil } - out := new(SandboxEgressRule) - in.DeepCopyInto(out) - return out -} - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *SandboxIngressRule) DeepCopyInto(out *SandboxIngressRule) { - *out = *in - if in.CIDRs != nil { - in, out := &in.CIDRs, &out.CIDRs - *out = make([]string, len(*in)) - copy(*out, *in) - } - if in.Ports != nil { - in, out := &in.Ports, &out.Ports - *out = make([]SandboxNetworkPolicyPort, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SandboxIngressRule. -func (in *SandboxIngressRule) DeepCopy() *SandboxIngressRule { - if in == nil { - return nil - } - out := new(SandboxIngressRule) + out := new(SandboxNetworkPolicyRule) in.DeepCopyInto(out) return out } @@ -383,30 +355,20 @@ func (in *SandboxIngressRule) DeepCopy() *SandboxIngressRule { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SandboxNetworkPolicy) DeepCopyInto(out *SandboxNetworkPolicy) { *out = *in - if in.AllowedEgress != nil { - in, out := &in.AllowedEgress, &out.AllowedEgress - *out = make([]SandboxEgressRule, len(*in)) + if in.Egress != nil { + in, out := &in.Egress, &out.Egress + *out = make([]SandboxNetworkPolicyRule, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } } - if in.AllowDNS != nil { - in, out := &in.AllowDNS, &out.AllowDNS - *out = new(bool) - **out = **in - } - if in.AllowedIngress != nil { - in, out := &in.AllowedIngress, &out.AllowedIngress - *out = make([]SandboxIngressRule, len(*in)) + if in.Ingress != nil { + in, out := &in.Ingress, &out.Ingress + *out = make([]SandboxNetworkPolicyRule, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } } - if in.Custom != nil { - in, out := &in.Custom, &out.Custom - *out = new(SandboxNetworkPolicyCustomRules) - (*in).DeepCopyInto(*out) - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SandboxNetworkPolicy. @@ -419,35 +381,6 @@ func (in *SandboxNetworkPolicy) DeepCopy() *SandboxNetworkPolicy { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *SandboxNetworkPolicyCustomRules) DeepCopyInto(out *SandboxNetworkPolicyCustomRules) { - *out = *in - if in.Ingress != nil { - in, out := &in.Ingress, &out.Ingress - *out = make([]networkingv1.NetworkPolicyIngressRule, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } - } - if in.Egress != nil { - in, out := &in.Egress, &out.Egress - *out = make([]networkingv1.NetworkPolicyEgressRule, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SandboxNetworkPolicyCustomRules. -func (in *SandboxNetworkPolicyCustomRules) DeepCopy() *SandboxNetworkPolicyCustomRules { - if in == nil { - return nil - } - out := new(SandboxNetworkPolicyCustomRules) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SandboxNetworkPolicyPort) DeepCopyInto(out *SandboxNetworkPolicyPort) { *out = *in diff --git a/pkg/common/types/sandbox.go b/pkg/common/types/sandbox.go index 7d5e295b..314ed609 100644 --- a/pkg/common/types/sandbox.go +++ b/pkg/common/types/sandbox.go @@ -90,20 +90,6 @@ func (car *CreateSandboxRequest) Validate() error { // validateNetworkPolicyOverride validates a per-session NetworkPolicy override from // CreateSandboxRequest. Unlike the CRD field (which has kubebuilder markers), this // comes in as raw JSON and bypasses API-server admission, so we validate it here. -func validateNetworkPolicyOverride(np *runtimev1alpha1.SandboxNetworkPolicy) error { - if np == nil { - return nil - } - switch np.Mode { - case runtimev1alpha1.NetworkPolicyModeNone, - runtimev1alpha1.NetworkPolicyModeRestricted, - runtimev1alpha1.NetworkPolicyModeCustom, - "": // empty is treated as None - default: - return fmt.Errorf("unknown mode %q: must be one of None, Restricted, Custom", np.Mode) - } - if np.Mode == runtimev1alpha1.NetworkPolicyModeCustom && np.Custom == nil { - return fmt.Errorf("custom must be set when mode is Custom") - } +func validateNetworkPolicyOverride(_ *runtimev1alpha1.SandboxNetworkPolicy) error { return nil } diff --git a/pkg/workloadmanager/handlers.go b/pkg/workloadmanager/handlers.go index ff52d767..c54129f2 100644 --- a/pkg/workloadmanager/handlers.go +++ b/pkg/workloadmanager/handlers.go @@ -25,7 +25,6 @@ import ( "github.com/gin-gonic/gin" networkingv1 "k8s.io/api/networking/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" k8stypes "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/dynamic" @@ -317,28 +316,16 @@ func (s *Server) handleDeleteSandbox(c *gin.Context) { } if sandbox.Kind == types.SandboxClaimsKind { - err = deleteSandboxClaim(c.Request.Context(), dynamicClient, sandbox.SandboxNamespace, sandbox.Name) - if err != nil { - if apierrors.IsNotFound(err) { - // Already deleted, consider as success - klog.Infof("sandbox claim %s/%s already deleted", sandbox.SandboxNamespace, sandbox.Name) - } else { - klog.Errorf("failed to delete sandbox claim %s/%s: %v", sandbox.SandboxNamespace, sandbox.Name, err) - respondError(c, http.StatusInternalServerError, "internal server error") - return - } + if err = deleteSandboxClaim(c.Request.Context(), dynamicClient, sandbox.SandboxNamespace, sandbox.Name); err != nil { + klog.Errorf("failed to delete sandbox claim %s/%s: %v", sandbox.SandboxNamespace, sandbox.Name, err) + respondError(c, http.StatusInternalServerError, "internal server error") + return } } else { - err = deleteSandbox(c.Request.Context(), dynamicClient, sandbox.SandboxNamespace, sandbox.Name) - if err != nil { - if apierrors.IsNotFound(err) { - // Already deleted, consider as success - klog.Infof("sandbox %s/%s already deleted", sandbox.SandboxNamespace, sandbox.Name) - } else { - klog.Errorf("failed to delete sandbox %s/%s: %v", sandbox.SandboxNamespace, sandbox.Name, err) - respondError(c, http.StatusInternalServerError, "internal server error") - return - } + if err = deleteSandbox(c.Request.Context(), dynamicClient, sandbox.SandboxNamespace, sandbox.Name); err != nil { + klog.Errorf("failed to delete sandbox %s/%s: %v", sandbox.SandboxNamespace, sandbox.Name, err) + respondError(c, http.StatusInternalServerError, "internal server error") + return } } diff --git a/pkg/workloadmanager/handlers_test.go b/pkg/workloadmanager/handlers_test.go index fa5edd78..7c48fb98 100644 --- a/pkg/workloadmanager/handlers_test.go +++ b/pkg/workloadmanager/handlers_test.go @@ -437,7 +437,7 @@ func TestHandleSandboxCreate(t *testing.T) { func TestHandleSandboxCreate_SessionNetworkPolicyOverride(t *testing.T) { gin.SetMode(gin.TestMode) - body := `{"name":"workload","namespace":"ns","networkPolicy":{"mode":"Restricted","allowDNS":false}}` + body := `{"name":"workload","namespace":"ns","networkPolicy":{"egress":[{"cidrs":["10.0.0.0/8"]}]}}` for _, kind := range []string{types.AgentRuntimeKind, types.CodeInterpreterKind} { t.Run(kind, func(t *testing.T) { @@ -472,9 +472,8 @@ func TestHandleSandboxCreate_SessionNetworkPolicyOverride(t *testing.T) { require.Equal(t, http.StatusOK, w.Code) require.NotNil(t, capturedNP, "session NetworkPolicy override must reach the builder") - assert.Equal(t, runtimev1alpha1.NetworkPolicyModeRestricted, capturedNP.Mode) - require.NotNil(t, capturedNP.AllowDNS) - assert.False(t, *capturedNP.AllowDNS, "AllowDNS=false from request body should propagate") + require.Len(t, capturedNP.Egress, 1) + assert.Equal(t, "10.0.0.0/8", capturedNP.Egress[0].CIDRs[0], "egress CIDR from request body should propagate") }) } } diff --git a/pkg/workloadmanager/networkpolicy_builder.go b/pkg/workloadmanager/networkpolicy_builder.go index f77dd3d8..e2211f1c 100644 --- a/pkg/workloadmanager/networkpolicy_builder.go +++ b/pkg/workloadmanager/networkpolicy_builder.go @@ -22,6 +22,7 @@ import ( networkingv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/klog/v2" ) // k8sNamespaceLabelKey is the label automatically applied to every Namespace by @@ -29,9 +30,11 @@ import ( // NamespaceSelector key avoids requiring users to manually label namespaces. const k8sNamespaceLabelKey = "kubernetes.io/metadata.name" -// buildNetworkPolicy returns a NetworkPolicy for the given sandbox, or nil when Mode=None. +// buildNetworkPolicy returns a deny-all NetworkPolicy for the given sandbox with +// explicit allow rules for router ingress and DNS egress, plus any additional rules +// from spec. Returns nil when spec is nil (no enforcement). func buildNetworkPolicy(sandboxName, namespace string, spec *runtimev1alpha1.SandboxNetworkPolicy, routerSelector map[string]string, routerNamespace string) *networkingv1.NetworkPolicy { - if spec == nil || spec.Mode == runtimev1alpha1.NetworkPolicyModeNone || spec.Mode == "" { + if spec == nil { return nil } @@ -57,37 +60,33 @@ func buildNetworkPolicy(sandboxName, namespace string, spec *runtimev1alpha1.San }, } - switch spec.Mode { - case runtimev1alpha1.NetworkPolicyModeRestricted: - np.Spec.Ingress = buildRestrictedIngressRules(spec, routerSelector, routerNamespace) - np.Spec.Egress = buildRestrictedEgressRules(spec) - case runtimev1alpha1.NetworkPolicyModeCustom: - if spec.Custom != nil { - np.Spec.Ingress = spec.Custom.Ingress - np.Spec.Egress = spec.Custom.Egress - } - } + np.Spec.Ingress = buildIngressRules(spec, routerSelector, routerNamespace) + np.Spec.Egress = buildEgressRules(spec) return np } -func buildRestrictedIngressRules(spec *runtimev1alpha1.SandboxNetworkPolicy, routerSelector map[string]string, routerNamespace string) []networkingv1.NetworkPolicyIngressRule { - // Router ingress is always allowed in Restricted mode. NamespaceSelector is set - // so the rule works even when the router runs in a namespace distinct from the - // sandbox (the common production layout: router in agentcube-system, sandboxes - // in user namespaces). +func buildIngressRules(spec *runtimev1alpha1.SandboxNetworkPolicy, routerSelector map[string]string, routerNamespace string) []networkingv1.NetworkPolicyIngressRule { + if routerNamespace == "" { + // Without a NamespaceSelector the peer only matches pods in the same namespace + // as the NetworkPolicy, defeating cross-namespace router ingress. This is a + // configuration error that must be caught loudly — silent wrong semantics here + // would make the sandbox unreachable in any multi-namespace layout. + klog.Fatalf("routerNamespace must not be empty: a missing NamespaceSelector restricts router ingress to the sandbox namespace only") + } + // Router ingress is always allowed. NamespaceSelector is set so the rule works + // when the router runs in a namespace distinct from the sandbox (the common + // production layout: router in agentcube-system, sandboxes in user namespaces). routerPeer := networkingv1.NetworkPolicyPeer{ PodSelector: &metav1.LabelSelector{MatchLabels: routerSelector}, - } - if routerNamespace != "" { - routerPeer.NamespaceSelector = &metav1.LabelSelector{ + NamespaceSelector: &metav1.LabelSelector{ MatchLabels: map[string]string{k8sNamespaceLabelKey: routerNamespace}, - } + }, } rules := []networkingv1.NetworkPolicyIngressRule{ {From: []networkingv1.NetworkPolicyPeer{routerPeer}}, } - for _, r := range spec.AllowedIngress { + for _, r := range spec.Ingress { rule := networkingv1.NetworkPolicyIngressRule{} for _, cidr := range r.CIDRs { rule.From = append(rule.From, networkingv1.NetworkPolicyPeer{ @@ -100,23 +99,20 @@ func buildRestrictedIngressRules(spec *runtimev1alpha1.SandboxNetworkPolicy, rou return rules } -func buildRestrictedEgressRules(spec *runtimev1alpha1.SandboxNetworkPolicy) []networkingv1.NetworkPolicyEgressRule { +func buildEgressRules(spec *runtimev1alpha1.SandboxNetworkPolicy) []networkingv1.NetworkPolicyEgressRule { var rules []networkingv1.NetworkPolicyEgressRule - // Allow DNS by default (unless explicitly disabled). - if spec.AllowDNS == nil || *spec.AllowDNS { - tcp := corev1.ProtocolTCP - udp := corev1.ProtocolUDP - dnsPort := intstr.FromInt32(53) - rules = append(rules, networkingv1.NetworkPolicyEgressRule{ - Ports: []networkingv1.NetworkPolicyPort{ - {Protocol: &udp, Port: &dnsPort}, - {Protocol: &tcp, Port: &dnsPort}, - }, - }) - } + tcp := corev1.ProtocolTCP + udp := corev1.ProtocolUDP + dnsPort := intstr.FromInt32(53) + rules = append(rules, networkingv1.NetworkPolicyEgressRule{ + Ports: []networkingv1.NetworkPolicyPort{ + {Protocol: &udp, Port: &dnsPort}, + {Protocol: &tcp, Port: &dnsPort}, + }, + }) - for _, r := range spec.AllowedEgress { + for _, r := range spec.Egress { rule := networkingv1.NetworkPolicyEgressRule{} for _, cidr := range r.CIDRs { rule.To = append(rule.To, networkingv1.NetworkPolicyPeer{ diff --git a/pkg/workloadmanager/networkpolicy_builder_test.go b/pkg/workloadmanager/networkpolicy_builder_test.go index 89679379..9aafecdb 100644 --- a/pkg/workloadmanager/networkpolicy_builder_test.go +++ b/pkg/workloadmanager/networkpolicy_builder_test.go @@ -26,7 +26,6 @@ import ( networkingv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" - "k8s.io/utils/ptr" ) var ( @@ -34,26 +33,15 @@ var ( testRouterNamespace = "agentcube-system" ) -func TestBuildNetworkPolicy_NoneMode(t *testing.T) { - // nil spec → no NP +func TestBuildNetworkPolicy_NilSpec(t *testing.T) { + // nil spec → no NP (no enforcement) np := buildNetworkPolicy("sandbox-1", "default", nil, testRouterSelector, testRouterNamespace) assert.Nil(t, np) - - // explicit None → no NP - np = buildNetworkPolicy("sandbox-1", "default", &runtimev1alpha1.SandboxNetworkPolicy{ - Mode: runtimev1alpha1.NetworkPolicyModeNone, - }, testRouterSelector, testRouterNamespace) - assert.Nil(t, np) - - // empty mode string → no NP - np = buildNetworkPolicy("sandbox-1", "default", &runtimev1alpha1.SandboxNetworkPolicy{}, testRouterSelector, testRouterNamespace) - assert.Nil(t, np) } func TestBuildNetworkPolicy_Metadata(t *testing.T) { - np := buildNetworkPolicy("my-sandbox", "my-ns", &runtimev1alpha1.SandboxNetworkPolicy{ - Mode: runtimev1alpha1.NetworkPolicyModeRestricted, - }, testRouterSelector, testRouterNamespace) + np := buildNetworkPolicy("my-sandbox", "my-ns", &runtimev1alpha1.SandboxNetworkPolicy{}, + testRouterSelector, testRouterNamespace) require.NotNil(t, np) assert.Equal(t, "my-sandbox", np.Name) @@ -64,10 +52,10 @@ func TestBuildNetworkPolicy_Metadata(t *testing.T) { assert.Contains(t, np.Spec.PolicyTypes, networkingv1.PolicyTypeEgress) } -func TestBuildNetworkPolicy_RestrictedDefaults(t *testing.T) { - np := buildNetworkPolicy("sb", "ns", &runtimev1alpha1.SandboxNetworkPolicy{ - Mode: runtimev1alpha1.NetworkPolicyModeRestricted, - }, testRouterSelector, testRouterNamespace) +func TestBuildNetworkPolicy_DefaultRules(t *testing.T) { + // Non-nil spec with no extra rules → router ingress + DNS egress + np := buildNetworkPolicy("sb", "ns", &runtimev1alpha1.SandboxNetworkPolicy{}, + testRouterSelector, testRouterNamespace) require.NotNil(t, np) // Ingress: only router rule by default @@ -90,20 +78,10 @@ func TestBuildNetworkPolicy_RestrictedDefaults(t *testing.T) { assert.True(t, ports[corev1.ProtocolTCP], "expected TCP DNS port") } -func TestBuildNetworkPolicy_RestrictedDNSDisabled(t *testing.T) { - np := buildNetworkPolicy("sb", "ns", &runtimev1alpha1.SandboxNetworkPolicy{ - Mode: runtimev1alpha1.NetworkPolicyModeRestricted, - AllowDNS: ptr.To(false), - }, testRouterSelector, testRouterNamespace) - require.NotNil(t, np) - assert.Empty(t, np.Spec.Egress, "expected no egress rules when DNS disabled and no AllowedEgress") -} - -func TestBuildNetworkPolicy_RestrictedAllowedEgress(t *testing.T) { +func TestBuildNetworkPolicy_Egress(t *testing.T) { tcp := corev1.ProtocolTCP np := buildNetworkPolicy("sb", "ns", &runtimev1alpha1.SandboxNetworkPolicy{ - Mode: runtimev1alpha1.NetworkPolicyModeRestricted, - AllowedEgress: []runtimev1alpha1.SandboxEgressRule{ + Egress: []runtimev1alpha1.SandboxNetworkPolicyRule{ { CIDRs: []string{"10.0.0.0/8"}, Ports: []runtimev1alpha1.SandboxNetworkPolicyPort{ @@ -124,10 +102,9 @@ func TestBuildNetworkPolicy_RestrictedAllowedEgress(t *testing.T) { assert.Equal(t, corev1.ProtocolTCP, *customRule.Ports[0].Protocol) } -func TestBuildNetworkPolicy_RestrictedAllowedIngress(t *testing.T) { +func TestBuildNetworkPolicy_Ingress(t *testing.T) { np := buildNetworkPolicy("sb", "ns", &runtimev1alpha1.SandboxNetworkPolicy{ - Mode: runtimev1alpha1.NetworkPolicyModeRestricted, - AllowedIngress: []runtimev1alpha1.SandboxIngressRule{ + Ingress: []runtimev1alpha1.SandboxNetworkPolicyRule{ {CIDRs: []string{"192.168.1.0/24"}}, }, }, testRouterSelector, testRouterNamespace) @@ -140,46 +117,10 @@ func TestBuildNetworkPolicy_RestrictedAllowedIngress(t *testing.T) { assert.Equal(t, "192.168.1.0/24", customRule.From[0].IPBlock.CIDR) } -func TestBuildNetworkPolicy_CustomMode(t *testing.T) { - tcp := corev1.ProtocolTCP - port80 := intstr.FromInt32(80) - np := buildNetworkPolicy("sb", "ns", &runtimev1alpha1.SandboxNetworkPolicy{ - Mode: runtimev1alpha1.NetworkPolicyModeCustom, - Custom: &runtimev1alpha1.SandboxNetworkPolicyCustomRules{ - Ingress: []networkingv1.NetworkPolicyIngressRule{ - { - From: []networkingv1.NetworkPolicyPeer{{IPBlock: &networkingv1.IPBlock{CIDR: "0.0.0.0/0"}}}, - Ports: []networkingv1.NetworkPolicyPort{{Protocol: &tcp, Port: &port80}}, - }, - }, - Egress: []networkingv1.NetworkPolicyEgressRule{ - {To: []networkingv1.NetworkPolicyPeer{{IPBlock: &networkingv1.IPBlock{CIDR: "0.0.0.0/0"}}}}, - }, - }, - }, testRouterSelector, testRouterNamespace) - require.NotNil(t, np) - - require.Len(t, np.Spec.Ingress, 1) - assert.Equal(t, "0.0.0.0/0", np.Spec.Ingress[0].From[0].IPBlock.CIDR) - require.Len(t, np.Spec.Egress, 1) - assert.Equal(t, "0.0.0.0/0", np.Spec.Egress[0].To[0].IPBlock.CIDR) -} - -func TestBuildNetworkPolicy_CustomModeNilCustom(t *testing.T) { - // Mode=Custom but Custom is nil → NP created with empty rules (deny all) - np := buildNetworkPolicy("sb", "ns", &runtimev1alpha1.SandboxNetworkPolicy{ - Mode: runtimev1alpha1.NetworkPolicyModeCustom, - }, testRouterSelector, testRouterNamespace) - require.NotNil(t, np) - assert.Empty(t, np.Spec.Ingress) - assert.Empty(t, np.Spec.Egress) -} - func TestBuildNetworkPolicy_RouterSelector(t *testing.T) { customSelector := map[string]string{"app": "my-custom-router", "tier": "proxy"} - np := buildNetworkPolicy("sb", "ns", &runtimev1alpha1.SandboxNetworkPolicy{ - Mode: runtimev1alpha1.NetworkPolicyModeRestricted, - }, customSelector, testRouterNamespace) + np := buildNetworkPolicy("sb", "ns", &runtimev1alpha1.SandboxNetworkPolicy{}, + customSelector, testRouterNamespace) require.NotNil(t, np) require.Len(t, np.Spec.Ingress, 1) @@ -192,9 +133,8 @@ func TestBuildNetworkPolicy_RouterCrossNamespace(t *testing.T) { // When routerNamespace is set, the router peer must carry a NamespaceSelector // using the standard kubernetes.io/metadata.name label so the rule works when // the sandbox is in a different namespace than the router. - np := buildNetworkPolicy("sb", "user-ns", &runtimev1alpha1.SandboxNetworkPolicy{ - Mode: runtimev1alpha1.NetworkPolicyModeRestricted, - }, testRouterSelector, "agentcube-system") + np := buildNetworkPolicy("sb", "user-ns", &runtimev1alpha1.SandboxNetworkPolicy{}, + testRouterSelector, "agentcube-system") require.NotNil(t, np) require.Len(t, np.Spec.Ingress, 1) @@ -204,41 +144,42 @@ func TestBuildNetworkPolicy_RouterCrossNamespace(t *testing.T) { assert.Equal(t, map[string]string{k8sNamespaceLabelKey: "agentcube-system"}, routerPeer.NamespaceSelector.MatchLabels) } -func TestBuildNetworkPolicy_RouterNamespaceEmpty(t *testing.T) { - // Empty routerNamespace falls back to same-namespace semantics (no NamespaceSelector). - np := buildNetworkPolicy("sb", "ns", &runtimev1alpha1.SandboxNetworkPolicy{ - Mode: runtimev1alpha1.NetworkPolicyModeRestricted, - }, testRouterSelector, "") +func TestBuildNetworkPolicy_RouterNamespaceAlwaysSet(t *testing.T) { + // NamespaceSelector must always be present on the router peer so that the rule + // works across namespaces. buildIngressRules fatals on empty namespace, + // so this test confirms a non-empty namespace produces the expected selector. + np := buildNetworkPolicy("sb", "ns", &runtimev1alpha1.SandboxNetworkPolicy{}, + testRouterSelector, testRouterNamespace) require.NotNil(t, np) require.Len(t, np.Spec.Ingress, 1) routerPeer := np.Spec.Ingress[0].From[0] - assert.Nil(t, routerPeer.NamespaceSelector) + require.NotNil(t, routerPeer.NamespaceSelector) + assert.Equal(t, testRouterNamespace, routerPeer.NamespaceSelector.MatchLabels[k8sNamespaceLabelKey]) } func TestBuildNetworkPolicy_SandboxNameLabel(t *testing.T) { - np := buildNetworkPolicy("unique-sandbox", "test-ns", &runtimev1alpha1.SandboxNetworkPolicy{ - Mode: runtimev1alpha1.NetworkPolicyModeRestricted, - }, testRouterSelector, testRouterNamespace) + np := buildNetworkPolicy("unique-sandbox", "test-ns", &runtimev1alpha1.SandboxNetworkPolicy{}, + testRouterSelector, testRouterNamespace) require.NotNil(t, np) - // Pod selector must target exactly this sandbox assert.Equal(t, metav1.LabelSelector{ MatchLabels: map[string]string{SandboxNameLabelKey: "unique-sandbox"}, }, np.Spec.PodSelector) } // TestEffectiveNetworkPolicy verifies "replace, not merge" semantics between -// template-level default and per-session override (promised to the maintainer). +// template-level default and per-session override. func TestEffectiveNetworkPolicy(t *testing.T) { template := &runtimev1alpha1.SandboxNetworkPolicy{ - Mode: runtimev1alpha1.NetworkPolicyModeRestricted, - AllowedEgress: []runtimev1alpha1.SandboxEgressRule{ + Egress: []runtimev1alpha1.SandboxNetworkPolicyRule{ {CIDRs: []string{"10.0.0.0/8"}}, }, } session := &runtimev1alpha1.SandboxNetworkPolicy{ - Mode: runtimev1alpha1.NetworkPolicyModeCustom, + Ingress: []runtimev1alpha1.SandboxNetworkPolicyRule{ + {CIDRs: []string{"192.168.0.0/16"}}, + }, } // session nil → template wins From 03d1407b3b17a0056d0c77aada6a5792e681383c Mon Sep 17 00:00:00 2001 From: Sanchit2662 Date: Fri, 24 Apr 2026 11:18:33 +0530 Subject: [PATCH 6/7] fix: regenerate deepcopy and CRD manifests after API simplification Signed-off-by: Sanchit2662 --- ...me.agentcube.volcano.sh_agentruntimes.yaml | 441 +----------------- ...agentcube.volcano.sh_codeinterpreters.yaml | 441 +----------------- .../runtime/v1alpha1/zz_generated.deepcopy.go | 54 +-- 3 files changed, 63 insertions(+), 873 deletions(-) diff --git a/manifests/charts/base/crds/runtime.agentcube.volcano.sh_agentruntimes.yaml b/manifests/charts/base/crds/runtime.agentcube.volcano.sh_agentruntimes.yaml index c49ab754..67cb96db 100644 --- a/manifests/charts/base/crds/runtime.agentcube.volcano.sh_agentruntimes.yaml +++ b/manifests/charts/base/crds/runtime.agentcube.volcano.sh_agentruntimes.yaml @@ -68,30 +68,25 @@ spec: NetworkPolicy defines the network isolation policy for sandbox pods created from this template. Defaults to Mode=None (no policy created). properties: - allowDNS: - description: |- - AllowDNS controls whether a DNS egress rule (port 53 UDP/TCP) is added when - Mode=Restricted. Defaults to true. - type: boolean - allowedEgress: - description: AllowedEgress lists additional egress allow rules - applied when Mode=Restricted. + egress: + description: Egress lists additional egress allow rules beyond + the default DNS rule. items: - description: SandboxEgressRule describes an egress allow - rule for a sandbox pod. + description: SandboxNetworkPolicyRule describes a single + ingress or egress allow rule for a sandbox pod. properties: cidrs: description: |- - CIDRs is the list of destination CIDR blocks. - Each entry must be a valid CIDR notation (e.g. "10.0.0.0/8" or "2001:db8::/32"). + CIDRs is the list of CIDR blocks for this rule (source for ingress, destination for egress). + Each entry must be valid CIDR notation (e.g. "10.0.0.0/8" or "2001:db8::/32"). items: pattern: ^(\d{1,3}\.){3}\d{1,3}\/\d{1,2}$|^[0-9a-fA-F:]+\/\d{1,3}$ type: string type: array x-kubernetes-list-type: atomic ports: - description: Ports restricts the rule to these destination - ports. An empty list matches all ports. + description: Ports restricts the rule to these ports. + An empty list matches all ports. items: description: SandboxNetworkPolicyPort describes a port for a sandbox network policy rule. @@ -113,26 +108,26 @@ spec: type: array type: object type: array - allowedIngress: + ingress: description: |- - AllowedIngress lists additional ingress allow rules when Mode=Restricted. - Router ingress is always permitted in Restricted mode regardless of this field. + Ingress lists additional ingress allow rules. + Router ingress is always permitted regardless of this field. items: - description: SandboxIngressRule describes an ingress allow - rule for a sandbox pod. + description: SandboxNetworkPolicyRule describes a single + ingress or egress allow rule for a sandbox pod. properties: cidrs: description: |- - CIDRs is the list of source CIDR blocks. - Each entry must be a valid CIDR notation (e.g. "10.0.0.0/8" or "2001:db8::/32"). + CIDRs is the list of CIDR blocks for this rule (source for ingress, destination for egress). + Each entry must be valid CIDR notation (e.g. "10.0.0.0/8" or "2001:db8::/32"). items: pattern: ^(\d{1,3}\.){3}\d{1,3}\/\d{1,2}$|^[0-9a-fA-F:]+\/\d{1,3}$ type: string type: array x-kubernetes-list-type: atomic ports: - description: Ports restricts the rule to these destination - ports. An empty list matches all ports. + description: Ports restricts the rule to these ports. + An empty list matches all ports. items: description: SandboxNetworkPolicyPort describes a port for a sandbox network policy rule. @@ -154,407 +149,7 @@ spec: type: array type: object type: array - custom: - description: Custom provides raw NetworkPolicy ingress/egress - rules, used only when Mode=Custom. - properties: - egress: - description: Egress is the list of egress rules. - items: - description: |- - NetworkPolicyEgressRule describes a particular set of traffic that is allowed out of pods - matched by a NetworkPolicySpec's podSelector. The traffic must match both ports and to. - This type is beta-level in 1.8 - properties: - ports: - description: |- - ports is a list of destination ports for outgoing traffic. - Each item in this list is combined using a logical OR. If this field is - empty or missing, this rule matches all ports (traffic not restricted by port). - If this field is present and contains at least one item, then this rule allows - traffic only if the traffic matches at least one port in the list. - items: - description: NetworkPolicyPort describes a port - to allow traffic on - properties: - endPort: - description: |- - endPort indicates that the range of ports from port to endPort if set, inclusive, - should be allowed by the policy. This field cannot be defined if the port field - is not defined or if the port field is defined as a named (string) port. - The endPort must be equal or greater than port. - format: int32 - type: integer - port: - anyOf: - - type: integer - - type: string - description: |- - port represents the port on the given protocol. This can either be a numerical or named - port on a pod. If this field is not provided, this matches all port names and - numbers. - If present, only traffic on the specified protocol AND port will be matched. - x-kubernetes-int-or-string: true - protocol: - description: |- - protocol represents the protocol (TCP, UDP, or SCTP) which traffic must match. - If not specified, this field defaults to TCP. - type: string - type: object - type: array - x-kubernetes-list-type: atomic - to: - description: |- - to is a list of destinations for outgoing traffic of pods selected for this rule. - Items in this list are combined using a logical OR operation. If this field is - empty or missing, this rule matches all destinations (traffic not restricted by - destination). If this field is present and contains at least one item, this rule - allows traffic only if the traffic matches at least one item in the to list. - items: - description: |- - NetworkPolicyPeer describes a peer to allow traffic to/from. Only certain combinations of - fields are allowed - properties: - ipBlock: - description: |- - ipBlock defines policy on a particular IPBlock. If this field is set then - neither of the other fields can be. - properties: - cidr: - description: |- - cidr is a string representing the IPBlock - Valid examples are "192.168.1.0/24" or "2001:db8::/64" - type: string - except: - description: |- - except is a slice of CIDRs that should not be included within an IPBlock - Valid examples are "192.168.1.0/24" or "2001:db8::/64" - Except values will be rejected if they are outside the cidr range - items: - type: string - type: array - x-kubernetes-list-type: atomic - required: - - cidr - type: object - namespaceSelector: - description: |- - namespaceSelector selects namespaces using cluster-scoped labels. This field follows - standard label selector semantics; if present but empty, it selects all namespaces. - - If podSelector is also set, then the NetworkPolicyPeer as a whole selects - the pods matching podSelector in the namespaces selected by namespaceSelector. - Otherwise it selects all pods in the namespaces selected by namespaceSelector. - properties: - matchExpressions: - description: matchExpressions is a list - of label selector requirements. The - requirements are ANDed. - items: - description: |- - A label selector requirement is a selector that contains values, a key, and an operator that - relates the key and values. - properties: - key: - description: key is the label key - that the selector applies to. - type: string - operator: - description: |- - operator represents a key's relationship to a set of values. - Valid operators are In, NotIn, Exists and DoesNotExist. - type: string - values: - description: |- - values is an array of string values. If the operator is In or NotIn, - the values array must be non-empty. If the operator is Exists or DoesNotExist, - the values array must be empty. This array is replaced during a strategic - merge patch. - items: - type: string - type: array - x-kubernetes-list-type: atomic - required: - - key - - operator - type: object - type: array - x-kubernetes-list-type: atomic - matchLabels: - additionalProperties: - type: string - description: |- - matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels - map is equivalent to an element of matchExpressions, whose key field is "key", the - operator is "In", and the values array contains only "value". The requirements are ANDed. - type: object - type: object - x-kubernetes-map-type: atomic - podSelector: - description: |- - podSelector is a label selector which selects pods. This field follows standard label - selector semantics; if present but empty, it selects all pods. - - If namespaceSelector is also set, then the NetworkPolicyPeer as a whole selects - the pods matching podSelector in the Namespaces selected by NamespaceSelector. - Otherwise it selects the pods matching podSelector in the policy's own namespace. - properties: - matchExpressions: - description: matchExpressions is a list - of label selector requirements. The - requirements are ANDed. - items: - description: |- - A label selector requirement is a selector that contains values, a key, and an operator that - relates the key and values. - properties: - key: - description: key is the label key - that the selector applies to. - type: string - operator: - description: |- - operator represents a key's relationship to a set of values. - Valid operators are In, NotIn, Exists and DoesNotExist. - type: string - values: - description: |- - values is an array of string values. If the operator is In or NotIn, - the values array must be non-empty. If the operator is Exists or DoesNotExist, - the values array must be empty. This array is replaced during a strategic - merge patch. - items: - type: string - type: array - x-kubernetes-list-type: atomic - required: - - key - - operator - type: object - type: array - x-kubernetes-list-type: atomic - matchLabels: - additionalProperties: - type: string - description: |- - matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels - map is equivalent to an element of matchExpressions, whose key field is "key", the - operator is "In", and the values array contains only "value". The requirements are ANDed. - type: object - type: object - x-kubernetes-map-type: atomic - type: object - type: array - x-kubernetes-list-type: atomic - type: object - type: array - ingress: - description: Ingress is the list of ingress rules. - items: - description: |- - NetworkPolicyIngressRule describes a particular set of traffic that is allowed to the pods - matched by a NetworkPolicySpec's podSelector. The traffic must match both ports and from. - properties: - from: - description: |- - from is a list of sources which should be able to access the pods selected for this rule. - Items in this list are combined using a logical OR operation. If this field is - empty or missing, this rule matches all sources (traffic not restricted by - source). If this field is present and contains at least one item, this rule - allows traffic only if the traffic matches at least one item in the from list. - items: - description: |- - NetworkPolicyPeer describes a peer to allow traffic to/from. Only certain combinations of - fields are allowed - properties: - ipBlock: - description: |- - ipBlock defines policy on a particular IPBlock. If this field is set then - neither of the other fields can be. - properties: - cidr: - description: |- - cidr is a string representing the IPBlock - Valid examples are "192.168.1.0/24" or "2001:db8::/64" - type: string - except: - description: |- - except is a slice of CIDRs that should not be included within an IPBlock - Valid examples are "192.168.1.0/24" or "2001:db8::/64" - Except values will be rejected if they are outside the cidr range - items: - type: string - type: array - x-kubernetes-list-type: atomic - required: - - cidr - type: object - namespaceSelector: - description: |- - namespaceSelector selects namespaces using cluster-scoped labels. This field follows - standard label selector semantics; if present but empty, it selects all namespaces. - - If podSelector is also set, then the NetworkPolicyPeer as a whole selects - the pods matching podSelector in the namespaces selected by namespaceSelector. - Otherwise it selects all pods in the namespaces selected by namespaceSelector. - properties: - matchExpressions: - description: matchExpressions is a list - of label selector requirements. The - requirements are ANDed. - items: - description: |- - A label selector requirement is a selector that contains values, a key, and an operator that - relates the key and values. - properties: - key: - description: key is the label key - that the selector applies to. - type: string - operator: - description: |- - operator represents a key's relationship to a set of values. - Valid operators are In, NotIn, Exists and DoesNotExist. - type: string - values: - description: |- - values is an array of string values. If the operator is In or NotIn, - the values array must be non-empty. If the operator is Exists or DoesNotExist, - the values array must be empty. This array is replaced during a strategic - merge patch. - items: - type: string - type: array - x-kubernetes-list-type: atomic - required: - - key - - operator - type: object - type: array - x-kubernetes-list-type: atomic - matchLabels: - additionalProperties: - type: string - description: |- - matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels - map is equivalent to an element of matchExpressions, whose key field is "key", the - operator is "In", and the values array contains only "value". The requirements are ANDed. - type: object - type: object - x-kubernetes-map-type: atomic - podSelector: - description: |- - podSelector is a label selector which selects pods. This field follows standard label - selector semantics; if present but empty, it selects all pods. - - If namespaceSelector is also set, then the NetworkPolicyPeer as a whole selects - the pods matching podSelector in the Namespaces selected by NamespaceSelector. - Otherwise it selects the pods matching podSelector in the policy's own namespace. - properties: - matchExpressions: - description: matchExpressions is a list - of label selector requirements. The - requirements are ANDed. - items: - description: |- - A label selector requirement is a selector that contains values, a key, and an operator that - relates the key and values. - properties: - key: - description: key is the label key - that the selector applies to. - type: string - operator: - description: |- - operator represents a key's relationship to a set of values. - Valid operators are In, NotIn, Exists and DoesNotExist. - type: string - values: - description: |- - values is an array of string values. If the operator is In or NotIn, - the values array must be non-empty. If the operator is Exists or DoesNotExist, - the values array must be empty. This array is replaced during a strategic - merge patch. - items: - type: string - type: array - x-kubernetes-list-type: atomic - required: - - key - - operator - type: object - type: array - x-kubernetes-list-type: atomic - matchLabels: - additionalProperties: - type: string - description: |- - matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels - map is equivalent to an element of matchExpressions, whose key field is "key", the - operator is "In", and the values array contains only "value". The requirements are ANDed. - type: object - type: object - x-kubernetes-map-type: atomic - type: object - type: array - x-kubernetes-list-type: atomic - ports: - description: |- - ports is a list of ports which should be made accessible on the pods selected for - this rule. Each item in this list is combined using a logical OR. If this field is - empty or missing, this rule matches all ports (traffic not restricted by port). - If this field is present and contains at least one item, then this rule allows - traffic only if the traffic matches at least one port in the list. - items: - description: NetworkPolicyPort describes a port - to allow traffic on - properties: - endPort: - description: |- - endPort indicates that the range of ports from port to endPort if set, inclusive, - should be allowed by the policy. This field cannot be defined if the port field - is not defined or if the port field is defined as a named (string) port. - The endPort must be equal or greater than port. - format: int32 - type: integer - port: - anyOf: - - type: integer - - type: string - description: |- - port represents the port on the given protocol. This can either be a numerical or named - port on a pod. If this field is not provided, this matches all port names and - numbers. - If present, only traffic on the specified protocol AND port will be matched. - x-kubernetes-int-or-string: true - protocol: - description: |- - protocol represents the protocol (TCP, UDP, or SCTP) which traffic must match. - If not specified, this field defaults to TCP. - type: string - type: object - type: array - x-kubernetes-list-type: atomic - type: object - type: array - type: object - mode: - default: None - description: |- - Mode controls the network policy enforcement level. - "None" (default): no NetworkPolicy is created. - "Restricted": deny-all baseline; DNS egress and router ingress are always allowed. - "Custom": use the raw Ingress/Egress rules in the Custom field. - enum: - - None - - Restricted - - Custom - type: string type: object - x-kubernetes-validations: - - message: custom must be set when mode is Custom - rule: self.mode != 'Custom' || (has(self.custom) && self.custom - != null) spec: description: Spec is the Pod's spec properties: diff --git a/manifests/charts/base/crds/runtime.agentcube.volcano.sh_codeinterpreters.yaml b/manifests/charts/base/crds/runtime.agentcube.volcano.sh_codeinterpreters.yaml index 2ada2f27..10a426fa 100644 --- a/manifests/charts/base/crds/runtime.agentcube.volcano.sh_codeinterpreters.yaml +++ b/manifests/charts/base/crds/runtime.agentcube.volcano.sh_codeinterpreters.yaml @@ -337,30 +337,25 @@ spec: NetworkPolicy defines the network isolation policy for sandbox pods created from this template. Defaults to Mode=None (no policy created). properties: - allowDNS: - description: |- - AllowDNS controls whether a DNS egress rule (port 53 UDP/TCP) is added when - Mode=Restricted. Defaults to true. - type: boolean - allowedEgress: - description: AllowedEgress lists additional egress allow rules - applied when Mode=Restricted. + egress: + description: Egress lists additional egress allow rules beyond + the default DNS rule. items: - description: SandboxEgressRule describes an egress allow - rule for a sandbox pod. + description: SandboxNetworkPolicyRule describes a single + ingress or egress allow rule for a sandbox pod. properties: cidrs: description: |- - CIDRs is the list of destination CIDR blocks. - Each entry must be a valid CIDR notation (e.g. "10.0.0.0/8" or "2001:db8::/32"). + CIDRs is the list of CIDR blocks for this rule (source for ingress, destination for egress). + Each entry must be valid CIDR notation (e.g. "10.0.0.0/8" or "2001:db8::/32"). items: pattern: ^(\d{1,3}\.){3}\d{1,3}\/\d{1,2}$|^[0-9a-fA-F:]+\/\d{1,3}$ type: string type: array x-kubernetes-list-type: atomic ports: - description: Ports restricts the rule to these destination - ports. An empty list matches all ports. + description: Ports restricts the rule to these ports. + An empty list matches all ports. items: description: SandboxNetworkPolicyPort describes a port for a sandbox network policy rule. @@ -382,26 +377,26 @@ spec: type: array type: object type: array - allowedIngress: + ingress: description: |- - AllowedIngress lists additional ingress allow rules when Mode=Restricted. - Router ingress is always permitted in Restricted mode regardless of this field. + Ingress lists additional ingress allow rules. + Router ingress is always permitted regardless of this field. items: - description: SandboxIngressRule describes an ingress allow - rule for a sandbox pod. + description: SandboxNetworkPolicyRule describes a single + ingress or egress allow rule for a sandbox pod. properties: cidrs: description: |- - CIDRs is the list of source CIDR blocks. - Each entry must be a valid CIDR notation (e.g. "10.0.0.0/8" or "2001:db8::/32"). + CIDRs is the list of CIDR blocks for this rule (source for ingress, destination for egress). + Each entry must be valid CIDR notation (e.g. "10.0.0.0/8" or "2001:db8::/32"). items: pattern: ^(\d{1,3}\.){3}\d{1,3}\/\d{1,2}$|^[0-9a-fA-F:]+\/\d{1,3}$ type: string type: array x-kubernetes-list-type: atomic ports: - description: Ports restricts the rule to these destination - ports. An empty list matches all ports. + description: Ports restricts the rule to these ports. + An empty list matches all ports. items: description: SandboxNetworkPolicyPort describes a port for a sandbox network policy rule. @@ -423,407 +418,7 @@ spec: type: array type: object type: array - custom: - description: Custom provides raw NetworkPolicy ingress/egress - rules, used only when Mode=Custom. - properties: - egress: - description: Egress is the list of egress rules. - items: - description: |- - NetworkPolicyEgressRule describes a particular set of traffic that is allowed out of pods - matched by a NetworkPolicySpec's podSelector. The traffic must match both ports and to. - This type is beta-level in 1.8 - properties: - ports: - description: |- - ports is a list of destination ports for outgoing traffic. - Each item in this list is combined using a logical OR. If this field is - empty or missing, this rule matches all ports (traffic not restricted by port). - If this field is present and contains at least one item, then this rule allows - traffic only if the traffic matches at least one port in the list. - items: - description: NetworkPolicyPort describes a port - to allow traffic on - properties: - endPort: - description: |- - endPort indicates that the range of ports from port to endPort if set, inclusive, - should be allowed by the policy. This field cannot be defined if the port field - is not defined or if the port field is defined as a named (string) port. - The endPort must be equal or greater than port. - format: int32 - type: integer - port: - anyOf: - - type: integer - - type: string - description: |- - port represents the port on the given protocol. This can either be a numerical or named - port on a pod. If this field is not provided, this matches all port names and - numbers. - If present, only traffic on the specified protocol AND port will be matched. - x-kubernetes-int-or-string: true - protocol: - description: |- - protocol represents the protocol (TCP, UDP, or SCTP) which traffic must match. - If not specified, this field defaults to TCP. - type: string - type: object - type: array - x-kubernetes-list-type: atomic - to: - description: |- - to is a list of destinations for outgoing traffic of pods selected for this rule. - Items in this list are combined using a logical OR operation. If this field is - empty or missing, this rule matches all destinations (traffic not restricted by - destination). If this field is present and contains at least one item, this rule - allows traffic only if the traffic matches at least one item in the to list. - items: - description: |- - NetworkPolicyPeer describes a peer to allow traffic to/from. Only certain combinations of - fields are allowed - properties: - ipBlock: - description: |- - ipBlock defines policy on a particular IPBlock. If this field is set then - neither of the other fields can be. - properties: - cidr: - description: |- - cidr is a string representing the IPBlock - Valid examples are "192.168.1.0/24" or "2001:db8::/64" - type: string - except: - description: |- - except is a slice of CIDRs that should not be included within an IPBlock - Valid examples are "192.168.1.0/24" or "2001:db8::/64" - Except values will be rejected if they are outside the cidr range - items: - type: string - type: array - x-kubernetes-list-type: atomic - required: - - cidr - type: object - namespaceSelector: - description: |- - namespaceSelector selects namespaces using cluster-scoped labels. This field follows - standard label selector semantics; if present but empty, it selects all namespaces. - - If podSelector is also set, then the NetworkPolicyPeer as a whole selects - the pods matching podSelector in the namespaces selected by namespaceSelector. - Otherwise it selects all pods in the namespaces selected by namespaceSelector. - properties: - matchExpressions: - description: matchExpressions is a list - of label selector requirements. The - requirements are ANDed. - items: - description: |- - A label selector requirement is a selector that contains values, a key, and an operator that - relates the key and values. - properties: - key: - description: key is the label key - that the selector applies to. - type: string - operator: - description: |- - operator represents a key's relationship to a set of values. - Valid operators are In, NotIn, Exists and DoesNotExist. - type: string - values: - description: |- - values is an array of string values. If the operator is In or NotIn, - the values array must be non-empty. If the operator is Exists or DoesNotExist, - the values array must be empty. This array is replaced during a strategic - merge patch. - items: - type: string - type: array - x-kubernetes-list-type: atomic - required: - - key - - operator - type: object - type: array - x-kubernetes-list-type: atomic - matchLabels: - additionalProperties: - type: string - description: |- - matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels - map is equivalent to an element of matchExpressions, whose key field is "key", the - operator is "In", and the values array contains only "value". The requirements are ANDed. - type: object - type: object - x-kubernetes-map-type: atomic - podSelector: - description: |- - podSelector is a label selector which selects pods. This field follows standard label - selector semantics; if present but empty, it selects all pods. - - If namespaceSelector is also set, then the NetworkPolicyPeer as a whole selects - the pods matching podSelector in the Namespaces selected by NamespaceSelector. - Otherwise it selects the pods matching podSelector in the policy's own namespace. - properties: - matchExpressions: - description: matchExpressions is a list - of label selector requirements. The - requirements are ANDed. - items: - description: |- - A label selector requirement is a selector that contains values, a key, and an operator that - relates the key and values. - properties: - key: - description: key is the label key - that the selector applies to. - type: string - operator: - description: |- - operator represents a key's relationship to a set of values. - Valid operators are In, NotIn, Exists and DoesNotExist. - type: string - values: - description: |- - values is an array of string values. If the operator is In or NotIn, - the values array must be non-empty. If the operator is Exists or DoesNotExist, - the values array must be empty. This array is replaced during a strategic - merge patch. - items: - type: string - type: array - x-kubernetes-list-type: atomic - required: - - key - - operator - type: object - type: array - x-kubernetes-list-type: atomic - matchLabels: - additionalProperties: - type: string - description: |- - matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels - map is equivalent to an element of matchExpressions, whose key field is "key", the - operator is "In", and the values array contains only "value". The requirements are ANDed. - type: object - type: object - x-kubernetes-map-type: atomic - type: object - type: array - x-kubernetes-list-type: atomic - type: object - type: array - ingress: - description: Ingress is the list of ingress rules. - items: - description: |- - NetworkPolicyIngressRule describes a particular set of traffic that is allowed to the pods - matched by a NetworkPolicySpec's podSelector. The traffic must match both ports and from. - properties: - from: - description: |- - from is a list of sources which should be able to access the pods selected for this rule. - Items in this list are combined using a logical OR operation. If this field is - empty or missing, this rule matches all sources (traffic not restricted by - source). If this field is present and contains at least one item, this rule - allows traffic only if the traffic matches at least one item in the from list. - items: - description: |- - NetworkPolicyPeer describes a peer to allow traffic to/from. Only certain combinations of - fields are allowed - properties: - ipBlock: - description: |- - ipBlock defines policy on a particular IPBlock. If this field is set then - neither of the other fields can be. - properties: - cidr: - description: |- - cidr is a string representing the IPBlock - Valid examples are "192.168.1.0/24" or "2001:db8::/64" - type: string - except: - description: |- - except is a slice of CIDRs that should not be included within an IPBlock - Valid examples are "192.168.1.0/24" or "2001:db8::/64" - Except values will be rejected if they are outside the cidr range - items: - type: string - type: array - x-kubernetes-list-type: atomic - required: - - cidr - type: object - namespaceSelector: - description: |- - namespaceSelector selects namespaces using cluster-scoped labels. This field follows - standard label selector semantics; if present but empty, it selects all namespaces. - - If podSelector is also set, then the NetworkPolicyPeer as a whole selects - the pods matching podSelector in the namespaces selected by namespaceSelector. - Otherwise it selects all pods in the namespaces selected by namespaceSelector. - properties: - matchExpressions: - description: matchExpressions is a list - of label selector requirements. The - requirements are ANDed. - items: - description: |- - A label selector requirement is a selector that contains values, a key, and an operator that - relates the key and values. - properties: - key: - description: key is the label key - that the selector applies to. - type: string - operator: - description: |- - operator represents a key's relationship to a set of values. - Valid operators are In, NotIn, Exists and DoesNotExist. - type: string - values: - description: |- - values is an array of string values. If the operator is In or NotIn, - the values array must be non-empty. If the operator is Exists or DoesNotExist, - the values array must be empty. This array is replaced during a strategic - merge patch. - items: - type: string - type: array - x-kubernetes-list-type: atomic - required: - - key - - operator - type: object - type: array - x-kubernetes-list-type: atomic - matchLabels: - additionalProperties: - type: string - description: |- - matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels - map is equivalent to an element of matchExpressions, whose key field is "key", the - operator is "In", and the values array contains only "value". The requirements are ANDed. - type: object - type: object - x-kubernetes-map-type: atomic - podSelector: - description: |- - podSelector is a label selector which selects pods. This field follows standard label - selector semantics; if present but empty, it selects all pods. - - If namespaceSelector is also set, then the NetworkPolicyPeer as a whole selects - the pods matching podSelector in the Namespaces selected by NamespaceSelector. - Otherwise it selects the pods matching podSelector in the policy's own namespace. - properties: - matchExpressions: - description: matchExpressions is a list - of label selector requirements. The - requirements are ANDed. - items: - description: |- - A label selector requirement is a selector that contains values, a key, and an operator that - relates the key and values. - properties: - key: - description: key is the label key - that the selector applies to. - type: string - operator: - description: |- - operator represents a key's relationship to a set of values. - Valid operators are In, NotIn, Exists and DoesNotExist. - type: string - values: - description: |- - values is an array of string values. If the operator is In or NotIn, - the values array must be non-empty. If the operator is Exists or DoesNotExist, - the values array must be empty. This array is replaced during a strategic - merge patch. - items: - type: string - type: array - x-kubernetes-list-type: atomic - required: - - key - - operator - type: object - type: array - x-kubernetes-list-type: atomic - matchLabels: - additionalProperties: - type: string - description: |- - matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels - map is equivalent to an element of matchExpressions, whose key field is "key", the - operator is "In", and the values array contains only "value". The requirements are ANDed. - type: object - type: object - x-kubernetes-map-type: atomic - type: object - type: array - x-kubernetes-list-type: atomic - ports: - description: |- - ports is a list of ports which should be made accessible on the pods selected for - this rule. Each item in this list is combined using a logical OR. If this field is - empty or missing, this rule matches all ports (traffic not restricted by port). - If this field is present and contains at least one item, then this rule allows - traffic only if the traffic matches at least one port in the list. - items: - description: NetworkPolicyPort describes a port - to allow traffic on - properties: - endPort: - description: |- - endPort indicates that the range of ports from port to endPort if set, inclusive, - should be allowed by the policy. This field cannot be defined if the port field - is not defined or if the port field is defined as a named (string) port. - The endPort must be equal or greater than port. - format: int32 - type: integer - port: - anyOf: - - type: integer - - type: string - description: |- - port represents the port on the given protocol. This can either be a numerical or named - port on a pod. If this field is not provided, this matches all port names and - numbers. - If present, only traffic on the specified protocol AND port will be matched. - x-kubernetes-int-or-string: true - protocol: - description: |- - protocol represents the protocol (TCP, UDP, or SCTP) which traffic must match. - If not specified, this field defaults to TCP. - type: string - type: object - type: array - x-kubernetes-list-type: atomic - type: object - type: array - type: object - mode: - default: None - description: |- - Mode controls the network policy enforcement level. - "None" (default): no NetworkPolicy is created. - "Restricted": deny-all baseline; DNS egress and router ingress are always allowed. - "Custom": use the raw Ingress/Egress rules in the Custom field. - enum: - - None - - Restricted - - Custom - type: string type: object - x-kubernetes-validations: - - message: custom must be set when mode is Custom - rule: self.mode != 'Custom' || (has(self.custom) && self.custom - != null) resources: description: |- Compute Resources required by this container. diff --git a/pkg/apis/runtime/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/runtime/v1alpha1/zz_generated.deepcopy.go index 90763098..991a82bf 100644 --- a/pkg/apis/runtime/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/runtime/v1alpha1/zz_generated.deepcopy.go @@ -325,33 +325,6 @@ func (in *CodeInterpreterStatus) DeepCopy() *CodeInterpreterStatus { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *SandboxNetworkPolicyRule) DeepCopyInto(out *SandboxNetworkPolicyRule) { - *out = *in - if in.CIDRs != nil { - in, out := &in.CIDRs, &out.CIDRs - *out = make([]string, len(*in)) - copy(*out, *in) - } - if in.Ports != nil { - in, out := &in.Ports, &out.Ports - *out = make([]SandboxNetworkPolicyPort, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SandboxNetworkPolicyRule. -func (in *SandboxNetworkPolicyRule) DeepCopy() *SandboxNetworkPolicyRule { - if in == nil { - return nil - } - out := new(SandboxNetworkPolicyRule) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SandboxNetworkPolicy) DeepCopyInto(out *SandboxNetworkPolicy) { *out = *in @@ -402,6 +375,33 @@ func (in *SandboxNetworkPolicyPort) DeepCopy() *SandboxNetworkPolicyPort { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SandboxNetworkPolicyRule) DeepCopyInto(out *SandboxNetworkPolicyRule) { + *out = *in + if in.CIDRs != nil { + in, out := &in.CIDRs, &out.CIDRs + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.Ports != nil { + in, out := &in.Ports, &out.Ports + *out = make([]SandboxNetworkPolicyPort, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SandboxNetworkPolicyRule. +func (in *SandboxNetworkPolicyRule) DeepCopy() *SandboxNetworkPolicyRule { + if in == nil { + return nil + } + out := new(SandboxNetworkPolicyRule) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SandboxTemplate) DeepCopyInto(out *SandboxTemplate) { *out = *in From 4edac2e05bf1447cbc62c56999ef14cd5a68ffd9 Mon Sep 17 00:00:00 2001 From: Sanchit2662 Date: Fri, 24 Apr 2026 21:38:53 +0530 Subject: [PATCH 7/7] fix: close NetworkPolicy enforcement gaps in sandbox creation Signed-off-by: Sanchit2662 --- pkg/workloadmanager/handlers.go | 40 ++++++++++++-- pkg/workloadmanager/k8s_client.go | 25 +++++++++ pkg/workloadmanager/workload_builder.go | 73 ++++++++++++++----------- 3 files changed, 100 insertions(+), 38 deletions(-) diff --git a/pkg/workloadmanager/handlers.go b/pkg/workloadmanager/handlers.go index c54129f2..d8280c7c 100644 --- a/pkg/workloadmanager/handlers.go +++ b/pkg/workloadmanager/handlers.go @@ -179,6 +179,31 @@ func setNetworkPolicyOwner(np *networkingv1.NetworkPolicy, apiVersion, kind, nam }} } +// createNetworkPolicyAndWorkload creates the NetworkPolicy (if non-nil) before the +// workload so the deny-all is in place before the CNI wires up the pod's network +// interface. If workload creation fails, the NP is cleaned up inline — the rollback +// defer in createSandbox is not yet registered at this point. +// On success, setNetworkPolicyOwner has populated networkPolicy.OwnerReferences. +func (s *Server) createNetworkPolicyAndWorkload(ctx context.Context, dynamicClient dynamic.Interface, sandbox *sandboxv1alpha1.Sandbox, sandboxClaim *extensionsv1alpha1.SandboxClaim, networkPolicy *networkingv1.NetworkPolicy) error { + if networkPolicy != nil { + if err := createNetworkPolicy(ctx, s.k8sClient.clientset, networkPolicy); err != nil { + return api.NewInternalError(err) + } + } + if err := createWorkload(ctx, dynamicClient, sandbox, sandboxClaim, networkPolicy); err != nil { + if networkPolicy != nil { + ctxTimeout, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + if delErr := deleteNetworkPolicy(ctxTimeout, s.k8sClient.clientset, networkPolicy.Namespace, networkPolicy.Name); delErr != nil { + klog.Warningf("NetworkPolicy %s/%s cleanup after workload create failure: %v", + networkPolicy.Namespace, networkPolicy.Name, delErr) + } + } + return err + } + return nil +} + // createSandbox performs sandbox creation and returns the response payload or an error with an HTTP status code. func (s *Server) createSandbox(ctx context.Context, dynamicClient dynamic.Interface, sandbox *sandboxv1alpha1.Sandbox, sandboxClaim *extensionsv1alpha1.SandboxClaim, networkPolicy *networkingv1.NetworkPolicy, sandboxEntry *sandboxEntry, resultChan <-chan SandboxStatusUpdate) (*types.CreateSandboxResponse, error) { // Store placeholder before creating, make sandbox/sandboxClaim GarbageCollection possible @@ -187,12 +212,12 @@ func (s *Server) createSandbox(ctx context.Context, dynamicClient dynamic.Interf return nil, api.NewInternalError(fmt.Errorf("store sandbox placeholder failed: %v", err)) } - if err := createWorkload(ctx, dynamicClient, sandbox, sandboxClaim, networkPolicy); err != nil { + if err := s.createNetworkPolicyAndWorkload(ctx, dynamicClient, sandbox, sandboxClaim, networkPolicy); err != nil { return nil, err } // Register rollback IMMEDIATELY after the sandbox/claim exists, before any - // subsequent failure path (NetworkPolicy create, ready wait, entrypoint probe, + // subsequent failure path (NetworkPolicy patch, ready wait, entrypoint probe, // store update). Rollback unconditionally attempts NP delete when networkPolicy // is non-nil; delete is idempotent (NotFound is swallowed), which also covers // the edge case where Create succeeded server-side but returned an error @@ -205,11 +230,14 @@ func (s *Server) createSandbox(ctx context.Context, dynamicClient dynamic.Interf s.sandboxRollback(sandboxClaim, dynamicClient, sandbox, networkPolicy, sandboxEntry) }() - // Create NetworkPolicy if one was built from the template spec. - // Uses the workloadmanager's own client (not the user client) since NP is infra. + // Patch the already-created NP to add the OwnerReference now that we have the UID. + // Best-effort GC safety net: if the sandbox is deleted out-of-band, GC cascade-deletes + // the NP. Explicit deletion in handleDeleteSandbox covers the normal path, so a + // patch failure is logged but not fatal. if networkPolicy != nil { - if err := createNetworkPolicy(ctx, s.k8sClient.clientset, networkPolicy); err != nil { - return nil, api.NewInternalError(err) + if err := patchNetworkPolicyOwner(ctx, s.k8sClient.clientset, networkPolicy); err != nil { + klog.Warningf("NetworkPolicy %s/%s owner patch failed, GC cascade will not apply: %v", + networkPolicy.Namespace, networkPolicy.Name, err) } } diff --git a/pkg/workloadmanager/k8s_client.go b/pkg/workloadmanager/k8s_client.go index 631e71e4..c3163f9d 100644 --- a/pkg/workloadmanager/k8s_client.go +++ b/pkg/workloadmanager/k8s_client.go @@ -18,6 +18,7 @@ package workloadmanager import ( "context" + "encoding/json" "fmt" "time" @@ -410,3 +411,27 @@ func deleteNetworkPolicy(ctx context.Context, clientset kubernetes.Interface, na } return nil } + +// patchNetworkPolicyOwner patches np's ownerReferences onto the already-created +// NetworkPolicy object in the cluster. Called after the sandbox UID is known so +// that Kubernetes GC can cascade-delete the NP if the owner is removed out-of-band. +func patchNetworkPolicyOwner(ctx context.Context, clientset kubernetes.Interface, np *networkingv1.NetworkPolicy) error { + if np == nil || len(np.OwnerReferences) == 0 { + return nil + } + patch := map[string]interface{}{ + "metadata": map[string]interface{}{ + "ownerReferences": np.OwnerReferences, + }, + } + data, err := json.Marshal(patch) + if err != nil { + return fmt.Errorf("marshal owner patch for network policy %s/%s: %w", np.Namespace, np.Name, err) + } + _, err = clientset.NetworkingV1().NetworkPolicies(np.Namespace).Patch( + ctx, np.Name, k8stypes.MergePatchType, data, metav1.PatchOptions{}) + if err != nil { + return fmt.Errorf("patch network policy %s/%s owner: %w", np.Namespace, np.Name, err) + } + return nil +} diff --git a/pkg/workloadmanager/workload_builder.go b/pkg/workloadmanager/workload_builder.go index c7c35011..d3bb6570 100644 --- a/pkg/workloadmanager/workload_builder.go +++ b/pkg/workloadmanager/workload_builder.go @@ -320,6 +320,45 @@ func buildCodeInterpreterEnvVars(templateEnv []corev1.EnvVar, authMode runtimev1 return envVars } +// buildWarmPoolObjects constructs the placeholder Sandbox and SandboxClaim for the +// warm-pool path. It also logs a warning when a NetworkPolicy is configured, because +// warm-pool pods do not carry SandboxNameLabelKey and enforcement cannot be applied. +func buildWarmPoolObjects(namespace, sandboxName, sessionID string, idleTimeout time.Duration, ci *runtimev1alpha1.CodeInterpreter, sessionNetworkPolicy *runtimev1alpha1.SandboxNetworkPolicy) (*sandboxv1alpha1.Sandbox, *extensionsv1alpha1.SandboxClaim) { + claim := buildSandboxClaimObject(&buildSandboxClaimParams{ + namespace: namespace, + name: sandboxName, + sandboxTemplateName: ci.Name, + sessionID: sessionID, + idleTimeout: idleTimeout, + ownerReference: &metav1.OwnerReference{ + APIVersion: ci.APIVersion, + Kind: ci.Kind, + Name: ci.Name, + UID: ci.UID, + }, + }) + sb := &sandboxv1alpha1.Sandbox{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: sandboxName, + Labels: map[string]string{SessionIdLabelKey: sessionID}, + }, + } + if ci.Spec.MaxSessionDuration != nil { + shutdownTime := metav1.NewTime(time.Now().Add(ci.Spec.MaxSessionDuration.Duration)) + sb.Spec.Lifecycle.ShutdownTime = &shutdownTime + } + // Warm-pool pods are pre-created and do not carry SandboxNameLabelKey, so a + // NetworkPolicy PodSelector would never match them. Skip NP creation entirely + // to avoid the illusion of enforcement. Future support requires agent-sandbox + // to propagate claim labels onto the bound pod (or a post-bind pod patch). + if effectiveNetworkPolicy(sessionNetworkPolicy, ci.Spec.Template.NetworkPolicy) != nil { + klog.Warningf("network policy configured for %s/%s but warm-pool pods do not carry %s: enforcement skipped for this session", + namespace, ci.Name, SandboxNameLabelKey) + } + return sb, claim +} + func buildSandboxByCodeInterpreter(namespace string, codeInterpreterName string, informer *Informers, routerSelector map[string]string, routerNamespace string, sessionNetworkPolicy *runtimev1alpha1.SandboxNetworkPolicy) (*sandboxv1alpha1.Sandbox, *extensionsv1alpha1.SandboxClaim, *networkingv1.NetworkPolicy, *sandboxEntry, error) { codeInterpreterKey := namespace + "/" + codeInterpreterName // TODO(hzxuzhonghu): make use of typed informer, so we don't need to do type conversion below @@ -373,39 +412,9 @@ func buildSandboxByCodeInterpreter(namespace string, codeInterpreterName string, } if codeInterpreterObj.Spec.WarmPoolSize != nil && *codeInterpreterObj.Spec.WarmPoolSize > 0 { - sandboxClaim := buildSandboxClaimObject(&buildSandboxClaimParams{ - namespace: namespace, - name: sandboxName, - sandboxTemplateName: codeInterpreterName, - sessionID: sessionID, - idleTimeout: idleTimeout, - ownerReference: &metav1.OwnerReference{ - APIVersion: codeInterpreterObj.APIVersion, - Kind: codeInterpreterObj.Kind, - Name: codeInterpreterObj.Name, - UID: codeInterpreterObj.UID, - }, - }) - simpleSandbox := &sandboxv1alpha1.Sandbox{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: namespace, - Name: sandboxName, - Labels: map[string]string{ - SessionIdLabelKey: sessionID, - }, - }, - } - if codeInterpreterObj.Spec.MaxSessionDuration != nil { - shutdownTime := metav1.NewTime(time.Now().Add(codeInterpreterObj.Spec.MaxSessionDuration.Duration)) - simpleSandbox.Spec.Lifecycle.ShutdownTime = &shutdownTime - } sandboxEntry.Kind = types.SandboxClaimsKind - np := buildNetworkPolicy(sandboxName, namespace, effectiveNetworkPolicy(sessionNetworkPolicy, codeInterpreterObj.Spec.Template.NetworkPolicy), routerSelector, routerNamespace) - // TODO: warm-pool pods do not currently carry SandboxNameLabelKey, so the - // NP PodSelector won't match them and enforcement is a no-op for this path. - // Fix requires agent-sandbox to propagate claim labels onto the bound pod, - // or a post-ready pod patch. Tracked in issue #216. - return simpleSandbox, sandboxClaim, np, sandboxEntry, nil + simpleSandbox, sandboxClaim := buildWarmPoolObjects(namespace, sandboxName, sessionID, idleTimeout, &codeInterpreterObj, sessionNetworkPolicy) + return simpleSandbox, sandboxClaim, nil, sandboxEntry, nil } // Normalize RuntimeClassName: if it's an empty string, set it to nil