agentgateway: fix retry overflow

[howardjohn]

Description

Quick fix for an edge case

Change Type

/kind fix

Changelog

NONE

Additional Notes

[Copilot]

Pull request overview

This PR fixes a retry attempts overflow issue by capping the maximum value at 255 to match the u8 storage limit in agentgateway. Additionally, it corrects a typo in an error message.

Key changes:

• Added min(*retry.Attempts, 255) capping in both translation paths to prevent u8 overflow
• Fixed typo in error message: "attemptes" → "attempts"

Reviewed changes

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

File	Description
pkg/agentgateway/translator/builtin_helpers.go	Caps retry attempts at 255 for Gateway API HTTPRouteRule retries to prevent u8 overflow
pkg/agentgateway/plugins/traffic_plugin.go	Caps retry attempts at 255 for AgentgatewayPolicy retries and fixes spelling error in validation message

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The validation logic is inconsistent with the actual behavior. The error message states "should be positive int32" suggesting values up to MaxInt32 are acceptable, but line 460 silently caps the value at 255. This creates a misleading validation experience where values between 256 and MaxInt32 pass validation but are then silently truncated. Consider either:

1. Updating the validation check to error when value > 255, OR
2. Updating the error message to indicate that values will be capped at 255

Suggested change

	return nil, fmt.Errorf("failed to parse retry attempts should be positive int32 (%d)", *a)
	return nil, fmt.Errorf("failed to parse retry attempts: must be a non-negative int32; values above 255 are capped (%d)", *a)

[Copilot]

The nolint comment "attempts asserted above" is misleading after adding the min() cap. While the assertion above ensures the value is within int32 range, the min() function now means the assertion doesn't guarantee the value is safe from overflow in the context of u8 storage. The comment should be updated to reflect that the min() cap is what makes this safe for u8 storage, not just the assertion.

Suggested change

	// Agentgateway stores this as a u8 so has a max of 255
	translatedRetry.Attempts = int32(min(*retry.Attempts, 255)) //nolint:gosec // G115: attempts asserted above
	// Agentgateway stores this as a u8 so has a max of 255; min() caps the value accordingly after the int32 range check above.
	translatedRetry.Attempts = int32(min(*retry.Attempts, 255)) //nolint:gosec // G115: value is range-checked and capped at 255 before casting

[Copilot]

The nolint comment "kubebuilder validation ensures 0 <= value, safe for int32" is misleading after adding the min() cap. While kubebuilder validation ensures non-negative values, the comment should clarify that the min() function is what prevents u8 overflow (capping at 255), not just the int32 safety from kubebuilder validation.

Suggested change

	tpRetry.Attempts = int32(min(*rule.Retry.Attempts, 255)) //nolint:gosec // G115: kubebuilder validation ensures 0 <= value, safe for int32
	tpRetry.Attempts = int32(min(*rule.Retry.Attempts, 255)) //nolint:gosec // G115: kubebuilder validation ensures Attempts is >= 0; min() caps it at 255, matching the u8 max and making this cast safe

[jenshu]

is there a unit test that can be added?

[npolshakova]

I think it needs to rerun with REFRESH_GOLDEN to regen the unit tests after the typo fix