feat: add --as/--as-group/--as-uid impersonation flags#114
Conversation
Signed-off-by: Mike Ditton <mike.ditton@vshn.net>
Signed-off-by: Mike Ditton <mike.ditton@vshn.net>
Signed-off-by: Mike Ditton <mike.ditton@vshn.net>
Signed-off-by: Mike Ditton <mike.ditton@vshn.net>
Signed-off-by: Mike Ditton <mike.ditton@vshn.net>
Signed-off-by: Mike Ditton <mike.ditton@vshn.net>
Signed-off-by: Mike Ditton <mike.ditton@vshn.net>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
💤 Files with no reviewable changes (2)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughAdds kubectl-style ChangesKubernetes impersonation support across CLI surfaces
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as crossplane CLI (e.g. trace, top, version)
participant ImpersonationFlags
participant ctrl as ctrl.GetConfig()
participant RestConfig
participant K8sClient as Kubernetes Client
User->>CLI: run command --as=jane --as-group=team
CLI->>ctrl: GetConfig()
ctrl-->>CLI: *rest.Config
CLI->>ImpersonationFlags: Apply(*rest.Config)
ImpersonationFlags->>RestConfig: set Impersonate.UserName, Groups, UID
CLI->>K8sClient: create client from impersonated config
K8sClient-->>CLI: client operating as impersonated identity
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/crossplane/trace/trace_test.go`:
- Around line 108-126: The impersonation parse tests across multiple files do
not follow the repository's table-driven test convention. Convert all four test
functions to table-driven structure:
cmd/crossplane/trace/trace_test.go#L108-L126 TestImpersonationFlagsParse should
use table cases with reason, args, and want fields for different flag
combinations; cmd/crossplane/top/top_test.go#L20-L35 TestImpersonationFlagsParse
should be refactored similarly but use diff-based assertions (cmp.Diff) instead
of manual error checks; cmd/crossplane/version/version_test.go#L25-L43 should
adopt table-driven structure with args and want fields for parse test cases;
cmd/crossplane/xpkg/impersonation_test.go#L25-L57 should fold the separate
install and update test logic into individual table-driven test cases (either as
a single table or two separate table-driven tests) with explicit reason fields
and expected impersonation field values. In all cases, use the standard pattern:
define a struct with test case fields, iterate over test cases, parse flags with
kong.New and Parse, and assert results using cmp.Diff for consistency with
repository coding guidelines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 691564f3-cff2-4a70-b07d-10209e1e553a
📒 Files selected for processing (14)
cmd/crossplane/common/kube/impersonation.gocmd/crossplane/common/kube/impersonation_test.gocmd/crossplane/completion/completion.gocmd/crossplane/completion/completion_test.gocmd/crossplane/top/top.gocmd/crossplane/top/top_test.gocmd/crossplane/trace/trace.gocmd/crossplane/trace/trace_test.gocmd/crossplane/version/fetch.gocmd/crossplane/version/version.gocmd/crossplane/version/version_test.gocmd/crossplane/xpkg/impersonation_test.gocmd/crossplane/xpkg/install.gocmd/crossplane/xpkg/update.go
Signed-off-by: Mike Ditton <mike.ditton@vshn.net>
adamwg
left a comment
There was a problem hiding this comment.
I've left a couple of notes on code organization and tests, but the actual implementation here looks great. Thanks for the contribution, @mikeshootzz!
There was a problem hiding this comment.
I'd prefer to put this functionality in internal rather than cmd/crossplane/common. It's CLI-specific, so I don't see any reason for it to be importable by external codebases, and we should get rid of this common directory eventually (see below).
This directory is a leftover from the CLI being in the core crossplane/crossplane repository (which intentionally doesn't have a pkg/ for exported packages) and wanting to expose some code for CLI-adjacent utilities like crossplane-diff to use. We'd like to start moving the code that lives here into either internal/ or pkg/ as appropriate to make it clear what can/should be imported externally (it's a bit of a smell to import code from cmd/).
There was a problem hiding this comment.
Thanks for letting me know! I'll move it to internal later today.
| "github.com/crossplane/cli/v2/cmd/crossplane/common/kube" | ||
| ) | ||
|
|
||
| func TestImpersonationFlagsParse(t *testing.T) { |
There was a problem hiding this comment.
I'm not sure this test is very valuable - it's mostly testing the internals of kong rather than our code.
If you do want to keep it, I'd suggest putting it alongside the ImpersonationFlags struct in the kube package rather than duplicating it in each command that uses the flags.
There was a problem hiding this comment.
Fair point. I'll fix that. Thanks!
redundant parse tests Signed-off-by: Mike Ditton <mike.ditton@vshn.net>
Description of your changes
Adds the kubectl-compatible impersonation flags as, as-group, and as-uid to every CLI command that talks to a cluster. This lets you run a command as a different user, group, or service account without switching your kubeconfig context.
The flags are defined once in a shared ImpersonationFlags struct and embedded into each command. After a command builds its client config, it applies the flags, which sets the impersonation fields on the request. This works for every command regardless of how it loads its config.
Commands covered: resource trace, cluster top, version, xpkg install, xpkg update, and the shell-completion predictors.
The behavior matches kubectl: the flag names and help text are the same, as-group is repeatable, there are no short forms, and validation is left to the API server.
Fixes #110
I have:
./nix.sh flake checkto ensure this PR is ready for review.Linked a PR or a docs tracking issue to document this change.Addedbackport release-x.ylabels to auto-backport this PR.Need help with this checklist? See the cheat sheet.