Validate merged SAA retry policy updates#10921
Open
fretz12 wants to merge 1 commit into
Open
Conversation
Clone SAA option protos before merging updates to avoid mutating original state. Reject nil top-level retry policy replacements and validate merged retry policy subfield updates across standalone and workflow activity update paths. Add shared field-mask helpers and regression coverage.
spkane31
approved these changes
Jul 7, 2026
spkane31
left a comment
spkane31
left a comment
Contributor
There was a problem hiding this comment.
Would be great to have functional tests too.
| } | ||
|
|
||
| func FieldMaskHasSubPath(fieldMaskPaths map[string]struct{}, path string) bool { | ||
| prefix := path + "." |
Contributor
There was a problem hiding this comment.
nit: should only add the . if it's not already there. Could do string.TrimSuffix(path, ".") + "."
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed?
Fixed SAA update-options handling to clone proto option state before merging updates, reject nil top-level retry policy replacements, and validate retry policy subfield updates after merging with existing state. Added shared field-mask helpers.
Why?
A subfield update could previously mutate aliased original options or produce an invalid merged retry policy, such as initial_interval > maximum_interval. A top-level retry_policy update with no policy could also slip through validation as nil, leading to inconsistent retry behavior or nil-deref risk.
How did you test it?