[v18] Add tctl acl commands for managing access list reviews#64587
[v18] Add tctl acl commands for managing access list reviews#64587r0mant wants to merge 1 commit intobranch/v18from
tctl acl commands for managing access list reviews#64587Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e24493fe6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if len(r.Spec.Notes) > reviewNotesMaxSizeBytes { | ||
| r.Spec.Notes = r.Spec.Notes[:reviewNotesMaxSizeBytes] |
There was a problem hiding this comment.
Truncate oversized review notes on rune boundaries
This byte-level truncation can split a multi-byte UTF-8 character when notes exceed the 200 KB limit, leaving ReviewSpec.Notes with invalid text. Because notes is later serialized as a protobuf string (api/proto/teleport/accesslist/v1/accesslist.proto), this can corrupt the final character or cause serialization failures for large non-ASCII notes; trimming at rune boundaries (or validating UTF-8 after truncation) avoids that edge case.
Useful? React with 👍 / 👎.
| for _, member := range strings.Split(c.removeMembers, ",") { | ||
| member = strings.TrimSpace(member) | ||
| if member != "" { | ||
| removeMembers = append(removeMembers, member) |
There was a problem hiding this comment.
Deduplicate --remove-members entries before submit
The CLI forwards duplicate member IDs unchanged, so input like --remove-members=a,a sends repeated removals in one review. The backend applies removals one-by-one and errors on deleting a missing member (lib/services/local/access_list.go), which means this request can fail after partial side effects (review creation and first removal), so deduplicating this slice client-side would prevent non-idempotent failures from simple input duplication.
Useful? React with 👍 / 👎.
Backport of #63672 to branch/v18.
Manual Test Plan
Test Environment
platform.teleport.sh
Test Cases
changelog: Add
tctl aclcommands for managing access list reviews