Add commands for manage users groups and access#339
Conversation
e82de0c to
13a6cc9
Compare
Signed-off-by: Ivan Zvyagintsev <ivan.zvyagintsev@flant.com>
13a6cc9 to
cabfa61
Compare
AlwxSin
left a comment
There was a problem hiding this comment.
There are lack of some test scenarios:
- no grant -> revoke round-trip
- no remove-member tests
- no tests for UserOperation creation
- no tests for group create/delete
| hash8, | ||
| ) | ||
|
|
||
| if len(name) > 253 { |
There was a problem hiding this comment.
The 8-char hash that guarantees uniqueness sits at the end of the name. I suggest add the hash in the end, after length check.
| return fmt.Errorf("setting members: %w", err) | ||
| } | ||
|
|
||
| _, err = groupClient.Update(ctx, obj, metav1.UpdateOptions{}) |
There was a problem hiding this comment.
Consider adding retry.RetryOnConflict from k8s.io/client-go/util/retry for case when someone do the update between .Get and .Update
| if err := unstructured.SetNestedSlice(obj.Object, members, "spec", "members"); err != nil { | ||
| return EnsureMemberResult{}, fmt.Errorf("setting members on group %q: %w", groupName, err) | ||
| } | ||
| if _, err := groupClient.Update(ctx, obj, metav1.UpdateOptions{}); err != nil { |
There was a problem hiding this comment.
Consider adding retry.RetryOnConflict from k8s.io/client-go/util/retry for case when someone do the update between .Get and .Update
| case iamtypes.KindGroup: | ||
| subjectPrincipal = subjectName | ||
| if _, err := dyn.Resource(iamtypes.GroupGVR).Get(cmd.Context(), subjectName, metav1.GetOptions{}); err != nil { | ||
| fmt.Fprintf(cmd.ErrOrStderr(), "Warning: local Group CR %q not found. Grant may target an external provider group.\n", subjectName) |
There was a problem hiding this comment.
Consider using check apierrors.IsNotFound(err) for Not found. If not, just print err (there are possible Forbidden, Timeout errors).
| } | ||
|
|
||
| func findViaGroup(inv *accessInventory, userName, grantGroupName string) string { | ||
| directGroups, transitiveGroups := inv.ResolveUserGroups(userName) |
There was a problem hiding this comment.
Consider using cache for groups. For now code walks over all groups even if .ResolveUserGroups returns all groups
| # Revoke a labels-scoped grant (must match the original --scope value) | ||
| d8 iam access revoke group admins --access-level Editor --scope labels=team=platform,tier=prod`) | ||
|
|
||
| func newRevokeCommand() *cobra.Command { |
There was a problem hiding this comment.
grant has --dry-run flag, but revoke hasn't. Consider adding dry run flag.
| return fmt.Errorf("deleting User %q: %w", name, err) | ||
| } | ||
|
|
||
| fmt.Fprintf(cmd.ErrOrStderr(), "Warning: user %q may still be referenced in Group memberships. Use \"d8 iam group remove-member\" to clean up.\n", name) |
There was a problem hiding this comment.
Shouldn't we automate this? There is a task for that, that we should remove deleted user from groups.
| if !managedOnly && !manualOnly { | ||
| return rows | ||
| } | ||
| out := rows[:0] |
There was a problem hiding this comment.
rows[:0] will mutate outer variable.
| out := rows[:0] | |
| out := make([]ruleRow, 0, len(rows)) |
| PortForwarding bool `json:"portForwarding"` | ||
| ManagedByD8 bool `json:"managedByD8Cli"` | ||
| Subjects []subjectJSON `json:"subjects"` | ||
| CreationTime time.Time `json:"creationTimestamp,omitempty"` |
There was a problem hiding this comment.
My linter tells me, that omitempty is useless against time.Time.
Either:
*time.Time json:"omitempty"time.Time json:"omitzero"
| return nil, cobra.ShellCompDirectiveError | ||
| } | ||
|
|
||
| ri := dyn.Resource(gvr) |
There was a problem hiding this comment.
Consider use dyn.Resource(gvr).Namespace(namespace) if namespace != ""
Description
New
d8 iamcommand tree for managing local users, groups, and access grants. Five top-level subcommands underinternal/iam/:d8 iam user—create/delete/reset-password/reset2fa/lock/unlock.d8 iam group—create/delete/add-member/remove-member.d8 iam access—grant/revoke.d8 iam get—user,group,rule.d8 iam list—users|user,groups|group,rules|rule.Read verbs are top-level (
d8 iam get user alice,d8 iam list users), not per-domain wrappers.Subjects are positional.
grant/revoketake-n/--namespace(repeatable, AR) or--scope cluster|all-namespaces|labels=K=V[,K2=V2,...](CAR withnamespaceSelector.labelSelector). Capabilities (--allow-scale,--port-forwarding) compose with any scope. d8-managed grants get a deterministic name, sogrant/revokeare idempotent.Password input is unified across
user createanduser reset-password: interactive (default),--password-stdin,--generate-password, or--password-hash. The CLI handles the format difference betweenUser.spec.password(base64-bcrypt) andUserOperation.spec.resetPassword.newPasswordHash(raw bcrypt).Shell completion covers command names, resource names, namespaces, access levels, scope values, rule refs, and output formats. Common k8s helpers (
PrintObject,NewDynamicClient,AddOutputFlag,CompleteResourceNames, ...) live ininternal/utilk8s/. The previously top-leveld8 userwas moved underd8 iam user.Why do we need it, and what problem does it solve?
Managing users, groups, and access in Deckhouse today means hand-crafting
User,Group,AuthorizationRule, andClusterAuthorizationRuleCRs and applying them viakubectl. There is no first-class CLI for inventory, "who has access to what", or safe revocation.d8 iamprovides:iam list users|groups(aggregated table) andiam get user|group <name>(direct grants, transitive group membership, inherited grants, effective summary, warnings for cycles / orphaned members / manually maintained rules), withSuperAdminwildcard capabilities surfaced as implicit;iam list rules/iam get rulewith reverse lookup of subjects to localUser/GroupCRs;grant/revokeWhy do we need it in the patch release (if we do)?
Not necessarily.
Changelog entries