api: tighten runtime CRD validation by googs1025 · Pull Request #382 · volcano-sh/agentcube

googs1025 · 2026-06-10T11:59:09Z

Summary

add CRD schema/CEL validation for runtime session durations and ordering
validate runtime target ports and path prefixes
enforce non-negative CodeInterpreter warm pool size and add generated CRD schema tests

Test Plan

GOCACHE=/private/tmp/agentcube-go-cache make gen-crd
GOCACHE=/private/tmp/agentcube-go-cache go test -gcflags=all=-l ./pkg/apis/runtime/v1alpha1 ./pkg/workloadmanager -count=1

volcano-sh-bot · 2026-06-10T11:59:16Z

[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 hzxuzhonghu for approval. 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

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds/updates Kubebuilder CRD validations for runtime APIs (durations, ports, warm pool sizing, and cross-field constraints) and introduces a Go test to assert the generated CRD schemas contain the expected OpenAPI/CEL validation rules.

Changes:

Add Format=duration and positive-duration CEL validations for sessionTimeout / maxSessionDuration.
Add min/max/regex validations for ports, path prefix, and warm pool size; add cross-field sessionTimeout <= maxSessionDuration validations.
Add tests that load the generated CRDs and assert the schema contains the expected validation constraints.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
pkg/apis/runtime/v1alpha1/crd_validation_test.go	New tests to verify generated CRD OpenAPI schema and CEL rules.
pkg/apis/runtime/v1alpha1/codeinterpreter_types.go	Adds Kubebuilder validation markers for CodeInterpreter spec fields and port fields.
pkg/apis/runtime/v1alpha1/agent_type.go	Adds Kubebuilder validation markers for AgentRuntime spec duration fields and cross-field constraint.
manifests/charts/base/crds/runtime.agentcube.volcano.sh_codeinterpreters.yaml	Regenerated CRD including the new schema constraints and CEL rules.
manifests/charts/base/crds/runtime.agentcube.volcano.sh_agentruntimes.yaml	Regenerated CRD including the new schema constraints and CEL rules.

googs1025 · 2026-06-10T14:40:06Z

+			name:    "CodeInterpreter",
+			crdFile: "runtime.agentcube.volcano.sh_codeinterpreters.yaml",
+			ports:   "ports",
+			rule:    "!has(self.sessionTimeout) || !has(self.maxSessionDuration) || self.sessionTimeout <= self.maxSessionDuration",
+		},


googs1025 · 2026-06-10T14:40:08Z

+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			specSchema := loadCRDSpecSchema(t, tt.crdFile)


googs1025 · 2026-06-10T14:40:10Z

+	path := filepath.Join("..", "..", "..", "..", "manifests", "charts", "base", "crds", crdFile)
+	data, err := os.ReadFile(path)


gemini-code-assist

Code Review

This pull request adds OpenAPI validation rules and CEL validations to the AgentRuntime and CodeInterpreter CRDs, ensuring correct port ranges, path prefixes, and session duration limits, along with a test suite to verify the generated schemas. However, the CEL validation rules contain critical bugs: comparing metav1.Duration fields (which are serialized as strings) directly using operators like <= or > results in lexicographical string comparisons or runtime type mismatch errors. To fix this, all duration fields must be explicitly wrapped in the duration() function (e.g., duration(self)) in the Go API definitions, the generated YAML manifests, and the corresponding unit tests.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

gemini-code-assist · 2026-06-10T12:00:58Z

 }

 // AgentRuntimeSpec describes how to create and manage agent runtime sandboxes.
+// +kubebuilder:validation:XValidation:rule="self.sessionTimeout <= self.maxSessionDuration",message="sessionTimeout must be less than or equal to maxSessionDuration"


In CEL, comparing string fields (which metav1.Duration is represented as in OpenAPI/JSON) directly using <= performs a lexicographical string comparison instead of a duration comparison. For example, "10h" <= "8h" would evaluate to true because "1" is lexicographically less than "8". To compare them correctly as durations, they must be parsed using the duration() function.

Suggested change

// +kubebuilder:validation:XValidation:rule="self.sessionTimeout <= self.maxSessionDuration",message="sessionTimeout must be less than or equal to maxSessionDuration"

// +kubebuilder:validation:XValidation:rule="duration(self.sessionTimeout) <= duration(self.maxSessionDuration)",message="sessionTimeout must be less than or equal to maxSessionDuration"

gemini-code-assist · 2026-06-10T12:00:58Z

 	// SessionTimeout describes the duration after which an inactive session will be terminated.
 	// +kubebuilder:validation:Required
+	// +kubebuilder:validation:Format=duration
+	// +kubebuilder:validation:XValidation:rule="self > duration('0s')",message="sessionTimeout must be greater than 0"


Since self is a string representing the duration, comparing it directly with a duration object (duration('0s')) will cause a CEL type mismatch runtime error, which will reject all API requests. We must parse self using duration(self).

Suggested change

// +kubebuilder:validation:XValidation:rule="self > duration('0s')",message="sessionTimeout must be greater than 0"

// +kubebuilder:validation:XValidation:rule="duration(self) > duration('0s')",message="sessionTimeout must be greater than 0"

gemini-code-assist · 2026-06-10T12:00:58Z

 	// After this duration, the session will be terminated no matter active or inactive.
 	// +kubebuilder:validation:Required
+	// +kubebuilder:validation:Format=duration
+	// +kubebuilder:validation:XValidation:rule="self > duration('0s')",message="maxSessionDuration must be greater than 0"


Since self is a string representing the duration, comparing it directly with a duration object (duration('0s')) will cause a CEL type mismatch runtime error, which will reject all API requests. We must parse self using duration(self).

Suggested change

// +kubebuilder:validation:XValidation:rule="self > duration('0s')",message="maxSessionDuration must be greater than 0"

// +kubebuilder:validation:XValidation:rule="duration(self) > duration('0s')",message="maxSessionDuration must be greater than 0"

gemini-code-assist · 2026-06-10T12:00:58Z

 }

 // CodeInterpreterSpec describes how to create and manage code-interpreter sandboxes.
+// +kubebuilder:validation:XValidation:rule="!has(self.sessionTimeout) || !has(self.maxSessionDuration) || self.sessionTimeout <= self.maxSessionDuration",message="sessionTimeout must be less than or equal to maxSessionDuration"


In CEL, comparing string fields (which metav1.Duration is represented as in OpenAPI/JSON) directly using <= performs a lexicographical string comparison instead of a duration comparison. To compare them correctly as durations, they must be parsed using the duration() function.

Suggested change

// +kubebuilder:validation:XValidation:rule="!has(self.sessionTimeout) || !has(self.maxSessionDuration) || self.sessionTimeout <= self.maxSessionDuration",message="sessionTimeout must be less than or equal to maxSessionDuration"

// +kubebuilder:validation:XValidation:rule="!has(self.sessionTimeout) || !has(self.maxSessionDuration) || duration(self.sessionTimeout) <= duration(self.maxSessionDuration)",message="sessionTimeout must be less than or equal to maxSessionDuration"

gemini-code-assist · 2026-06-10T12:00:58Z

 	// session will be terminated. Any sandbox that has not received requests within
 	// this duration is eligible for cleanup.
+	// +kubebuilder:validation:Format=duration
+	// +kubebuilder:validation:XValidation:rule="self > duration('0s')",message="sessionTimeout must be greater than 0"


Since self is a string representing the duration, comparing it directly with a duration object (duration('0s')) will cause a CEL type mismatch runtime error, which will reject all API requests. We must parse self using duration(self).

Suggested change

// +kubebuilder:validation:XValidation:rule="self > duration('0s')",message="sessionTimeout must be greater than 0"

// +kubebuilder:validation:XValidation:rule="duration(self) > duration('0s')",message="sessionTimeout must be greater than 0"

gemini-code-assist · 2026-06-10T12:00:59Z

+            x-kubernetes-validations:
+            - message: sessionTimeout must be less than or equal to maxSessionDuration
+              rule: '!has(self.sessionTimeout) || !has(self.maxSessionDuration) ||
+                self.sessionTimeout <= self.maxSessionDuration'


In CEL, comparing string fields directly using <= performs a lexicographical string comparison instead of a duration comparison. To compare them correctly as durations, they must be parsed using the duration() function.

duration(self.sessionTimeout) <= duration(self.maxSessionDuration)'

gemini-code-assist · 2026-06-10T12:00:59Z

+			name:    "AgentRuntime",
+			crdFile: "runtime.agentcube.volcano.sh_agentruntimes.yaml",
+			ports:   "targetPort",
+			rule:    "self.sessionTimeout <= self.maxSessionDuration",


Update the expected validation rule to match the correct duration-based comparison.

Suggested change

rule: "self.sessionTimeout <= self.maxSessionDuration",

rule: "duration(self.sessionTimeout) <= duration(self.maxSessionDuration)",

gemini-code-assist · 2026-06-10T12:00:59Z

+			name:    "CodeInterpreter",
+			crdFile: "runtime.agentcube.volcano.sh_codeinterpreters.yaml",
+			ports:   "ports",
+			rule:    "!has(self.sessionTimeout) || !has(self.maxSessionDuration) || self.sessionTimeout <= self.maxSessionDuration",


Update the expected validation rule to match the correct duration-based comparison.

Suggested change

rule: "!has(self.sessionTimeout) || !has(self.maxSessionDuration) || self.sessionTimeout <= self.maxSessionDuration",

rule: "!has(self.sessionTimeout) || !has(self.maxSessionDuration) || duration(self.sessionTimeout) <= duration(self.maxSessionDuration)",

gemini-code-assist · 2026-06-10T12:00:59Z

+
+			sessionTimeoutSchema := nestedMap(t, specSchema, "properties", "sessionTimeout")
+			assertString(t, sessionTimeoutSchema, "format", "duration")
+			assertHasValidationRule(t, sessionTimeoutSchema, "self > duration('0s')")


Update the expected validation rule to match the correct duration-based comparison.

Suggested change

assertHasValidationRule(t, sessionTimeoutSchema, "self > duration('0s')")

assertHasValidationRule(t, sessionTimeoutSchema, "duration(self) > duration('0s')")

gemini-code-assist · 2026-06-10T12:00:59Z

+
+			maxSessionDurationSchema := nestedMap(t, specSchema, "properties", "maxSessionDuration")
+			assertString(t, maxSessionDurationSchema, "format", "duration")
+			assertHasValidationRule(t, maxSessionDurationSchema, "self > duration('0s')")


Update the expected validation rule to match the correct duration-based comparison.

Suggested change

assertHasValidationRule(t, maxSessionDurationSchema, "self > duration('0s')")

assertHasValidationRule(t, maxSessionDurationSchema, "duration(self) > duration('0s')")

codecov-commenter · 2026-06-10T12:09:29Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 57.96%. Comparing base (524e55e) to head (311d738).
⚠️ Report is 121 commits behind head on main.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #382       +/-   ##
===========================================
+ Coverage   47.57%   57.96%   +10.39%     
===========================================
  Files          30       34        +4     
  Lines        2819     3181      +362     
===========================================
+ Hits         1341     1844      +503     
+ Misses       1338     1153      -185     
- Partials      140      184       +44

Flag	Coverage Δ
unittests	`57.96% <ø> (+10.39%)`	⬆️

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.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

googs1025 · 2026-06-10T14:35:24Z

+	return nestedMap(t, crd,
+		"spec", "versions", "0", "schema", "openAPIV3Schema", "properties", "spec")


googs1025 · 2026-06-10T14:35:25Z

 }

 // AgentRuntimeSpec describes how to create and manage agent runtime sandboxes.
+// +kubebuilder:validation:XValidation:rule="duration(self.sessionTimeout) <= duration(self.maxSessionDuration)",message="sessionTimeout must be less than or equal to maxSessionDuration"


Signed-off-by: CYJiang <googs1025@gmail.com>

Copilot AI review requested due to automatic review settings June 10, 2026 11:59

volcano-sh-bot requested review from YaoZengzeng and acsoto June 10, 2026 11:59

volcano-sh-bot added the size/L label Jun 10, 2026

googs1025 mentioned this pull request Jun 10, 2026

api: tighten runtime CRD validation googs1025/agentcube#1

Closed

Copilot AI reviewed Jun 10, 2026

View reviewed changes

gemini-code-assist Bot reviewed Jun 10, 2026

View reviewed changes

googs1025 force-pushed the add-crd-validation branch from ee6a37f to f5ec701 Compare June 10, 2026 12:03

googs1025 force-pushed the add-crd-validation branch from f5ec701 to c608717 Compare June 10, 2026 14:20

Copilot AI review requested due to automatic review settings June 10, 2026 14:20

Copilot AI reviewed Jun 10, 2026

View reviewed changes

api: tighten runtime CRD validation

311d738

Signed-off-by: CYJiang <googs1025@gmail.com>

googs1025 force-pushed the add-crd-validation branch from c608717 to 311d738 Compare June 10, 2026 14:34

		path := filepath.Join("..", "..", "..", "..", "manifests", "charts", "base", "crds", crdFile)
		data, err := os.ReadFile(path)

	// +kubebuilder:validation:XValidation:rule="self.sessionTimeout <= self.maxSessionDuration",message="sessionTimeout must be less than or equal to maxSessionDuration"
	// +kubebuilder:validation:XValidation:rule="duration(self.sessionTimeout) <= duration(self.maxSessionDuration)",message="sessionTimeout must be less than or equal to maxSessionDuration"

	rule: "self.sessionTimeout <= self.maxSessionDuration",
	rule: "duration(self.sessionTimeout) <= duration(self.maxSessionDuration)",

	rule: "!has(self.sessionTimeout) \|\| !has(self.maxSessionDuration) \|\| self.sessionTimeout <= self.maxSessionDuration",
	rule: "!has(self.sessionTimeout) \|\| !has(self.maxSessionDuration) \|\| duration(self.sessionTimeout) <= duration(self.maxSessionDuration)",

	assertHasValidationRule(t, sessionTimeoutSchema, "self > duration('0s')")
	assertHasValidationRule(t, sessionTimeoutSchema, "duration(self) > duration('0s')")

	assertHasValidationRule(t, maxSessionDurationSchema, "self > duration('0s')")
	assertHasValidationRule(t, maxSessionDurationSchema, "duration(self) > duration('0s')")

		return nestedMap(t, crd,
		"spec", "versions", "0", "schema", "openAPIV3Schema", "properties", "spec")

Conversation

googs1025 commented Jun 10, 2026

Summary

Test Plan

Uh oh!

volcano-sh-bot commented Jun 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

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!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

codecov-commenter commented Jun 10, 2026 •

edited

Loading