e2b add network by ZhaoQing7892 · Pull Request #616 · openkruise/agents

ZhaoQing7892 · 2026-07-08T09:23:03Z

No description provided.

kruise-bot · 2026-07-08T09:23:08Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign zmberg for approval by writing /assign @zmberg in a comment. For more information see:The Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

codecov · 2026-07-08T09:28:31Z

Codecov Report

❌ Patch coverage is 73.17881% with 81 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.86%. Comparing base (e549e5b) to head (ce68f6f).
⚠️ Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
pkg/servers/e2b/network.go	39.28%	34 Missing ⚠️
pkg/sandbox-manager/infra/sandboxcr/network.go	84.79%	16 Missing and 10 partials ⚠️
pkg/servers/e2b/create.go	73.33%	6 Missing and 2 partials ⚠️
pkg/cache/index.go	0.00%	7 Missing ⚠️
pkg/servers/e2b/services.go	25.00%	4 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #616      +/-   ##
==========================================
- Coverage   80.08%   79.86%   -0.22%     
==========================================
  Files         229      232       +3     
  Lines       17831    18145     +314     
==========================================
+ Hits        14280    14492     +212     
- Misses       2969     3053      +84     
- Partials      582      600      +18

Flag	Coverage Δ
unittests	`79.86% <73.17%> (-0.22%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

zmberg · 2026-07-10T03:49:27Z

Could you please share the official E2D documentation corresponding to this feature?

furykerry · 2026-07-10T11:23:31Z

 	Request(ctx context.Context, method, path string, port int, body io.Reader) (*http.Response, error) // Make a request to the Sandbox
 	CSIMount(ctx context.Context, driver string, request string) error                                  // request is string config for csi.NodePublishVolumeRequest
 	CreateCheckpoint(ctx context.Context, opts CreateCheckpointOptions) (string, error)
+	CreateSandboxNetwork(ctx context.Context, network SandboxNetworkConfig) error // Create TrafficPolicy CR for the sandbox


consider change the func name from XXXSandboxNetwork to XXXNetworkPolicy

furykerry · 2026-07-10T11:29:23Z

+	tpList := &agentsv1alpha1.TrafficPolicyList{}
+	if err := k8sClient.List(ctx, tpList,
+		client.InNamespace(namespace),
+		client.MatchingLabels{labelSandboxID: sandboxID},


plz add an index for TrafficPolicy using sandboxID, so that trafficpolicy list can be more efficient

furykerry · 2026-07-10T11:35:06Z

+	}
+	if len(tpList.Items) > 0 {
+		tp := &tpList.Items[0]
+		if tp.Spec.Egress != nil {


what about the ingress setting?

furykerry · 2026-07-10T11:41:02Z

 	log.Info("sandbox deleted", "id", id)
+
+	// Cleanup network CRs (TrafficPolicy) associated with the sandbox.
+	if err := sbx.DeleteSandboxNetwork(r.Context()); err != nil {


it is not necessary to delete trafficpolicy manually, k8s gc controller will delete the trafficpolicy anyway

deleted DeleteSandboxNetwork

furykerry · 2026-07-10T11:42:09Z

+}
+
+// DeleteSandboxNetwork deletes the TrafficPolicy CR associated with the sandbox.
+func (s *Sandbox) DeleteSandboxNetwork(ctx context.Context) error {


DeleteSandboxNetwork is not necessary

deleted DeleteSandboxNetwork

furykerry · 2026-07-10T11:46:38Z

+			Priority: 1000,
+			Selector: metav1.LabelSelector{
+				MatchLabels: map[string]string{
+					labelSandboxID: sandboxID,


consider using uid of sandbox CR as the matching label, sandbox id may be too long for label value

using MergeFrom + patch of method ensureSandboxUIDLabel()

furykerry · 2026-07-10T11:48:56Z

+const allTrafficCIDR = "0.0.0.0/0"
+
+// isCIDROrIP returns true if the entry is a valid CIDR or bare IP address.
+func isCIDROrIP(entry string) bool {


isCIDROrIP allTrafficCIDR/defaultDenyCIDR is duplicated func, consider extract them to a util package in pkg/utils/network

furykerry · 2026-07-10T11:55:24Z

+
+	// Inject sandbox-id label into pod template so that TrafficPolicy CRs can
+	// select this pod via label selector.
+	infra.MergePodLabels(sbx, map[string]string{


consider move the label assignment to L416, so no extra map is required

using MergeFrom + patch of method ensureSandboxUIDLabel()

furykerry · 2026-07-10T12:00:45Z

Could you please share the official E2D documentation corresponding to this feature?

https://e2b.dev/docs/network/internet-access

furykerry · 2026-07-10T12:10:43Z

+		for _, cidr := range allowOutCIDRs {
+			allowPeers = append(allowPeers, agentsv1alpha1.TrafficPolicyPeer{CIDR: cidr})
+		}
+		for _, fqdn := range allowOutDomains {


Issue: The E2B documentation states: "When any domain is used, the default nameserver 8.8.8.8 is automatically allowed to ensure proper DNS resolution." The PR's buildTrafficPolicy function does not implement this automatic DNS allowlist behavior. When a user specifies allowOut: ["api.example.com"] with a default-deny, the sandbox will be unable to resolve DNS because 8.8.8.8 is not in the allow list.

Consider: In buildTrafficPolicy, when allowOutDomains is non-empty and the default-deny rule is being added, automatically include 8.8.8.8/32 in the allow list

furykerry · 2026-07-10T12:12:28Z

+			AllowOut: request.Network.AllowOut,
+			DenyOut:  request.Network.DenyOut,
+		}); netErr != nil {
+			log.Error(netErr, "failed to create network CRs, sandbox is still usable")


network creation failure should fail the sandbox creation entirely for security-sensitive workloads.

furykerry · 2026-07-10T14:08:32Z

+	TemplateID          string                `json:"templateID"`
+	Timeout             int                   `json:"timeout,omitempty"`
+	AutoPause           bool                  `json:"autoPause,omitempty"`
+	AllowInternetAccess *bool                 `json:"allow_internet_access,omitempty"`


Issue: The new field uses snake_case json:"allow_internet_access", while every other E2B model field in this repo is camelCase (templateID, envVars, metadata) and the sibling network fields are camelCase (allowOut, denyOut). E2B's HTTP API surface for these options is camelCase (allowInternetAccess); allow_internet_access/allow_out are the Python SDK parameter names, not the wire format.

Recommendation: Confirm the exact wire format the SDK sends; align to the repo/E2B convention to avoid the toggle silently no-op'ing when the field fails to bind:

furykerry · 2026-07-10T14:11:13Z

+
+type SandboxNetworkUpdateConfig struct {
+	AllowInternetAccess *bool `json:"allow_internet_access,omitempty"`
+	*SandboxNetworkConfig


Issue: Embedding *SandboxNetworkConfig relies on encoding/json's field-promotion + auto-allocation. It works for the current shapes but is fragile and non-obvious for an API request model.
Recommendation: Use explicit fields for clarity and stability:

kruise-bot requested review from furykerry and zmberg July 8, 2026 09:23

kruise-bot added the size/XXL label Jul 8, 2026

ZhaoQing7892 force-pushed the add_network branch from b3f0513 to 06f9fcd Compare July 8, 2026 10:10

AiRanthem mentioned this pull request Jul 8, 2026

fix: stabilize quota anti-drift primary-loss test under -race #617

Merged

ZhaoQing7892 force-pushed the add_network branch from 06f9fcd to 96de133 Compare July 10, 2026 05:04

furykerry reviewed Jul 10, 2026

View reviewed changes

ZhaoQing7892 added 2 commits July 13, 2026 13:31

e2b add network

d90409e

update network

0ff3672

ZhaoQing7892 force-pushed the add_network branch from 96de133 to 8b5b7a6 Compare July 13, 2026 08:29

update for reviews

ce68f6f

ZhaoQing7892 force-pushed the add_network branch from 8b5b7a6 to ce68f6f Compare July 14, 2026 01:35

Uh oh!

Conversation

ZhaoQing7892 commented Jul 8, 2026

Uh oh!

kruise-bot commented Jul 8, 2026

Uh oh!

codecov Bot commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

zmberg commented Jul 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

furykerry commented Jul 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov Bot commented Jul 8, 2026 •

edited

Loading