diff --git a/internal/commands/pro_platform_device_groups.go b/internal/commands/pro_platform_device_groups.go index 49abb6c..d8a7cda 100644 --- a/internal/commands/pro_platform_device_groups.go +++ b/internal/commands/pro_platform_device_groups.go @@ -3,14 +3,20 @@ package commands import ( + "context" + "encoding/json" "fmt" + "net/http" + "net/url" "os" + "strings" "github.com/spf13/cobra" platformgen "github.com/Jamf-Concepts/jamf-cli/internal/commands/platform/generated" "github.com/Jamf-Concepts/jamf-cli/internal/platform" "github.com/Jamf-Concepts/jamf-cli/internal/registry" + "github.com/Jamf-Concepts/jamfplatform-go-sdk/jamfplatform" "github.com/Jamf-Concepts/jamfplatform-go-sdk/jamfplatform/devicegroups" ) @@ -21,12 +27,26 @@ func newPlatformDeviceGroupsCmd(cliCtx *registry.CLIContext) *cobra.Command { Long: "Create and manage unified device groups via the Jamf Platform API. Requires platform gateway auth.", } - // Generated CRUD and member ops (list, create, get, patch, delete, members, patch-members) + // Generated CRUD: list, create. Skip --name-using ops; replaced below with + // handwritten versions that add --device-type for COMPUTER/MOBILE disambiguation. + needsType := map[string]bool{ + "get": true, "delete": true, "patch": true, "members": true, "patch-members": true, + } for _, sub := range platformgen.NewDeviceGroupsCmd(cliCtx).Commands() { + if needsType[sub.Name()] { + continue + } cmd.AddCommand(sub) } - // Business logic: upsert apply and ergonomic member mutations + // Name-based CRUD with --device-type disambiguation + cmd.AddCommand(newPDGGetCmd(cliCtx)) + cmd.AddCommand(newPDGDeleteCmd(cliCtx)) + cmd.AddCommand(newPDGPatchCmd(cliCtx)) + cmd.AddCommand(newPDGMembersCmd(cliCtx)) + cmd.AddCommand(newPDGPatchMembersCmd(cliCtx)) + + // Business logic: upsert and ergonomic member mutations cmd.AddCommand(newPDGApplyCmd(cliCtx)) cmd.AddCommand(newPDGAddMembersCmd(cliCtx)) cmd.AddCommand(newPDGRemoveMembersCmd(cliCtx)) @@ -34,6 +54,239 @@ func newPlatformDeviceGroupsCmd(cliCtx *registry.CLIContext) *cobra.Command { return cmd } +// pdgListPath returns the tenant-prefixed list endpoint for device groups. +func pdgListPath(c *jamfplatform.Client) string { + return "/api/device-groups/v1/tenant/" + url.PathEscape(c.Transport().TenantID()) + "/device-groups" +} + +// pdgResolveID resolves a device group name to its ID, optionally filtering by +// deviceType ("COMPUTER" or "MOBILE"). When deviceType is empty the lookup +// searches all groups; if two groups share a name the call errors with a hint +// to add --device-type. +func pdgResolveID(ctx context.Context, c *jamfplatform.Client, name, deviceType string) (string, error) { + filter := "" + if deviceType != "" { + filter = fmt.Sprintf(`deviceType=="%s"`, deviceType) + } + return platform.ResolveIDByNameFiltered(ctx, c, pdgListPath(c), name, filter) +} + +// normalizeDeviceTypeFlag uppercases the value and validates it is COMPUTER, +// MOBILE, or empty. Returns the normalized value and an error if invalid. +func normalizeDeviceTypeFlag(t string) (string, error) { + upper := strings.ToUpper(t) + if upper != "" && upper != "COMPUTER" && upper != "MOBILE" { + return "", fmt.Errorf("--device-type must be COMPUTER or MOBILE (got %q)", t) + } + return upper, nil +} + +// resolvePDGTarget normalizes --device-type, then resolves the group ID from +// either --name or a positional ID argument. +func resolvePDGTarget(ctx context.Context, cliCtx *registry.CLIContext, args []string, nameFlag, deviceTypeFlag string) (string, error) { + dt, err := normalizeDeviceTypeFlag(deviceTypeFlag) + if err != nil { + return "", err + } + if nameFlag != "" { + return pdgResolveID(ctx, cliCtx.PlatformSDKClient, nameFlag, dt) + } + if len(args) == 1 { + return args[0], nil + } + return "", fmt.Errorf("provide a positional ID or --name") +} + +// pdgItemPath returns the item-level endpoint for a device group ID. +func pdgItemPath(c *jamfplatform.Client, id string) string { + return pdgListPath(c) + "/" + url.PathEscape(id) +} + +func newPDGGetCmd(cliCtx *registry.CLIContext) *cobra.Command { + var nameFlag, deviceTypeFlag string + cmd := &cobra.Command{ + Use: "get ", + Short: "Get a device group by ID", + Long: "Retrieve a specific device group by its ID", + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if err := requirePlatformClient(cliCtx); err != nil { + return err + } + id, err := resolvePDGTarget(cmd.Context(), cliCtx, args, nameFlag, deviceTypeFlag) + if err != nil { + return err + } + var result any + if err := cliCtx.PlatformSDKClient.Transport().DoExpect(cmd.Context(), http.MethodGet, pdgItemPath(cliCtx.PlatformSDKClient, id), nil, http.StatusOK, &result); err != nil { + return fmt.Errorf("get: %w", err) + } + if result == nil { + return nil + } + b, err := json.MarshalIndent(result, "", " ") + if err != nil { + return err + } + return cliCtx.Output.PrintRaw(b) + }, + } + cmd.Flags().StringVar(&nameFlag, "name", "", "Resolve target by name instead of ID") + cmd.Flags().StringVar(&deviceTypeFlag, "device-type", "", "Narrow --name lookup by device type: COMPUTER or MOBILE") + return cmd +} + +func newPDGDeleteCmd(cliCtx *registry.CLIContext) *cobra.Command { + var nameFlag, deviceTypeFlag string + var yes bool + cmd := &cobra.Command{ + Use: "delete ", + Short: "Delete a device group", + Long: "Delete an existing device group", + Annotations: map[string]string{"jamf:destructive": "true"}, + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if err := requirePlatformClient(cliCtx); err != nil { + return err + } + id, err := resolvePDGTarget(cmd.Context(), cliCtx, args, nameFlag, deviceTypeFlag) + if err != nil { + return err + } + if err := platform.ConfirmAction("delete", id, yes); err != nil { + return err + } + if err := cliCtx.PlatformSDKClient.Transport().DoExpect(cmd.Context(), http.MethodDelete, pdgItemPath(cliCtx.PlatformSDKClient, id), nil, http.StatusNoContent, nil); err != nil { + return fmt.Errorf("delete: %w", err) + } + return nil + }, + } + cmd.Flags().StringVar(&nameFlag, "name", "", "Resolve target by name instead of ID") + cmd.Flags().StringVar(&deviceTypeFlag, "device-type", "", "Narrow --name lookup by device type: COMPUTER or MOBILE") + cmd.Flags().BoolVar(&yes, "yes", false, "Skip confirmation prompt") + return cmd +} + +func newPDGPatchCmd(cliCtx *registry.CLIContext) *cobra.Command { + var nameFlag, deviceTypeFlag, bodyFile string + var setFlags []string + var scaffoldFlag bool + cmd := &cobra.Command{ + Use: "patch ", + Short: "Update a device group", + Long: "Update an existing device group", + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if scaffoldFlag { + return printScaffold(map[string]any{ + "criteria": []any{}, + "description": "", + "name": "", + }) + } + if err := requirePlatformClient(cliCtx); err != nil { + return err + } + id, err := resolvePDGTarget(cmd.Context(), cliCtx, args, nameFlag, deviceTypeFlag) + if err != nil { + return err + } + body, err := platform.ReadBody(bodyFile, setFlags) + if err != nil { + return err + } + if err := cliCtx.PlatformSDKClient.Transport().DoExpect(cmd.Context(), http.MethodPatch, pdgItemPath(cliCtx.PlatformSDKClient, id), body, http.StatusNoContent, nil); err != nil { + return fmt.Errorf("patch: %w", err) + } + return nil + }, + } + cmd.Flags().StringVar(&nameFlag, "name", "", "Resolve target by name instead of ID") + cmd.Flags().StringVar(&deviceTypeFlag, "device-type", "", "Narrow --name lookup by device type: COMPUTER or MOBILE") + cmd.Flags().StringVar(&bodyFile, "file", "", "Path to JSON file containing the request body") + cmd.Flags().StringArrayVar(&setFlags, "set", nil, "Override body values (key=value, repeatable, supports nested.keys)") + cmd.Flags().BoolVar(&scaffoldFlag, "scaffold", false, "Print an example request body and exit") + return cmd +} + +func newPDGMembersCmd(cliCtx *registry.CLIContext) *cobra.Command { + var nameFlag, deviceTypeFlag string + cmd := &cobra.Command{ + Use: "members ", + Short: "Get group members", + Long: "Retrieve all members of a device group", + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if err := requirePlatformClient(cliCtx); err != nil { + return err + } + id, err := resolvePDGTarget(cmd.Context(), cliCtx, args, nameFlag, deviceTypeFlag) + if err != nil { + return err + } + path := pdgItemPath(cliCtx.PlatformSDKClient, id) + "/members" + var result any + if err := cliCtx.PlatformSDKClient.Transport().DoExpect(cmd.Context(), http.MethodGet, path, nil, http.StatusOK, &result); err != nil { + return fmt.Errorf("members: %w", err) + } + if result == nil { + return nil + } + b, err := json.MarshalIndent(result, "", " ") + if err != nil { + return err + } + return cliCtx.Output.PrintRaw(b) + }, + } + cmd.Flags().StringVar(&nameFlag, "name", "", "Resolve target by name instead of ID") + cmd.Flags().StringVar(&deviceTypeFlag, "device-type", "", "Narrow --name lookup by device type: COMPUTER or MOBILE") + return cmd +} + +func newPDGPatchMembersCmd(cliCtx *registry.CLIContext) *cobra.Command { + var nameFlag, deviceTypeFlag, bodyFile string + var setFlags []string + var scaffoldFlag bool + cmd := &cobra.Command{ + Use: "patch-members ", + Short: "Update device group members", + Long: "Add devices to or remove devices from a static device group. Cannot be used with smart groups.", + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if scaffoldFlag { + return printScaffold(map[string]any{ + "added": []any{}, + "removed": []any{}, + }) + } + if err := requirePlatformClient(cliCtx); err != nil { + return err + } + id, err := resolvePDGTarget(cmd.Context(), cliCtx, args, nameFlag, deviceTypeFlag) + if err != nil { + return err + } + path := pdgItemPath(cliCtx.PlatformSDKClient, id) + "/members" + body, err := platform.ReadBody(bodyFile, setFlags) + if err != nil { + return err + } + if err := cliCtx.PlatformSDKClient.Transport().DoExpect(cmd.Context(), http.MethodPatch, path, body, http.StatusNoContent, nil); err != nil { + return fmt.Errorf("patch-members: %w", err) + } + return nil + }, + } + cmd.Flags().StringVar(&nameFlag, "name", "", "Resolve target by name instead of ID") + cmd.Flags().StringVar(&deviceTypeFlag, "device-type", "", "Narrow --name lookup by device type: COMPUTER or MOBILE") + cmd.Flags().StringVar(&bodyFile, "file", "", "Path to JSON file containing the request body") + cmd.Flags().StringArrayVar(&setFlags, "set", nil, "Override body values (key=value, repeatable, supports nested.keys)") + cmd.Flags().BoolVar(&scaffoldFlag, "scaffold", false, "Print an example request body and exit") + return cmd +} + func newPDGApplyCmd(cliCtx *registry.CLIContext) *cobra.Command { var ( fromFile string @@ -65,8 +318,9 @@ func newPDGApplyCmd(cliCtx *registry.CLIContext) *cobra.Command { return fmt.Errorf("input must include a 'name' field") } - dg := devicegroups.New(cliCtx.PlatformSDKClient) - id, resolveErr := dg.ResolveDeviceGroupIDByName(ctx, createReq.Name) + // Use deviceType from the input JSON to disambiguate when a COMPUTER + // and MOBILE group share the same name. + id, resolveErr := pdgResolveID(ctx, cliCtx.PlatformSDKClient, createReq.Name, string(createReq.DeviceType)) if resolveErr != nil && !platform.IsNotFound(resolveErr) { return resolveErr } @@ -128,6 +382,7 @@ func deviceGroupScaffold() *devicegroups.DeviceGroupCreateRepresentationV1 { func newPDGAddMembersCmd(cliCtx *registry.CLIContext) *cobra.Command { var ids []string + var deviceTypeFlag string cmd := &cobra.Command{ Use: "add-members ", Short: "Add devices to a static group", @@ -136,12 +391,15 @@ func newPDGAddMembersCmd(cliCtx *registry.CLIContext) *cobra.Command { if err := requirePlatformClient(cliCtx); err != nil { return err } + dt, err := normalizeDeviceTypeFlag(deviceTypeFlag) + if err != nil { + return err + } if len(ids) == 0 { return fmt.Errorf("at least one --id is required") } ctx := cmd.Context() - dg := devicegroups.New(cliCtx.PlatformSDKClient) - groupID, err := dg.ResolveDeviceGroupIDByName(ctx, args[0]) + groupID, err := pdgResolveID(ctx, cliCtx.PlatformSDKClient, args[0], dt) if err != nil { return err } @@ -156,11 +414,13 @@ func newPDGAddMembersCmd(cliCtx *registry.CLIContext) *cobra.Command { }, } cmd.Flags().StringSliceVar(&ids, "id", nil, "Device ID to add (repeatable)") + cmd.Flags().StringVar(&deviceTypeFlag, "device-type", "", "Narrow name lookup by device type: COMPUTER or MOBILE") return cmd } func newPDGRemoveMembersCmd(cliCtx *registry.CLIContext) *cobra.Command { var ids []string + var deviceTypeFlag string cmd := &cobra.Command{ Use: "remove-members ", Short: "Remove devices from a static group", @@ -169,12 +429,15 @@ func newPDGRemoveMembersCmd(cliCtx *registry.CLIContext) *cobra.Command { if err := requirePlatformClient(cliCtx); err != nil { return err } + dt, err := normalizeDeviceTypeFlag(deviceTypeFlag) + if err != nil { + return err + } if len(ids) == 0 { return fmt.Errorf("at least one --id is required") } ctx := cmd.Context() - dg := devicegroups.New(cliCtx.PlatformSDKClient) - groupID, err := dg.ResolveDeviceGroupIDByName(ctx, args[0]) + groupID, err := pdgResolveID(ctx, cliCtx.PlatformSDKClient, args[0], dt) if err != nil { return err } @@ -189,5 +452,6 @@ func newPDGRemoveMembersCmd(cliCtx *registry.CLIContext) *cobra.Command { }, } cmd.Flags().StringSliceVar(&ids, "id", nil, "Device ID to remove (repeatable)") + cmd.Flags().StringVar(&deviceTypeFlag, "device-type", "", "Narrow name lookup by device type: COMPUTER or MOBILE") return cmd } diff --git a/internal/output/list_hint_test.go b/internal/output/list_hint_test.go index 30fa4ea..8031bee 100644 --- a/internal/output/list_hint_test.go +++ b/internal/output/list_hint_test.go @@ -94,7 +94,7 @@ func TestListHint_PrintRawJSONFastPath_TopLevelArray(t *testing.T) { // Top-level JSON array of 60 items, no projector → fast path triggers. var b bytes.Buffer b.WriteString("[") - for i := 0; i < 60; i++ { + for i := range 60 { if i > 0 { b.WriteString(",") } diff --git a/internal/platform/resolve_generic.go b/internal/platform/resolve_generic.go index e5d8aae..285d53d 100644 --- a/internal/platform/resolve_generic.go +++ b/internal/platform/resolve_generic.go @@ -28,10 +28,31 @@ import ( // "name", "title", and "displayName" properties in that order. The ID is read // from "id" (and falls back to "blueprintId", "groupId", "deviceId" for // resources that use a non-standard ID field). +// +// Returns an error when multiple items share the name (ambiguous match within +// a single page). Use ResolveIDByNameFiltered to narrow the lookup first. func ResolveIDByName(ctx context.Context, client *jamfplatform.Client, listPath string, name string) (string, error) { + return ResolveIDByNameFiltered(ctx, client, listPath, name, "") +} + +// ResolveIDByNameFiltered is like ResolveIDByName but narrows the server-side +// results with an RSQL filter expression appended as ?filter= before the +// name walk begins. Pass an empty string for no additional filtering. +// +// Example: ResolveIDByNameFiltered(ctx, c, path, "My Group", `deviceType=="COMPUTER"`) +func ResolveIDByNameFiltered(ctx context.Context, client *jamfplatform.Client, listPath string, name string, filter string) (string, error) { if name == "" { return "", fmt.Errorf("empty name") } + if filter != "" { + q := url.Values{} + q.Set("filter", filter) + sep := "?" + if u, err := url.Parse(listPath); err == nil && u.RawQuery != "" { + sep = "&" + } + listPath = listPath + sep + q.Encode() + } const pageSize = 100 @@ -44,12 +65,10 @@ func ResolveIDByName(ctx context.Context, client *jamfplatform.Client, listPath return "", fmt.Errorf("listing %s: %w", listPath, err) } items, paged := extractItems(raw) - for _, item := range items { - if matchesName(item, name) { - if id := extractID(item); id != "" { - return id, nil - } - } + if id, err := firstMatch(items, name); err != nil { + return "", err + } else if id != "" { + return id, nil } // Paginate only if the first response signalled more pages. @@ -69,12 +88,10 @@ func ResolveIDByName(ctx context.Context, client *jamfplatform.Client, listPath return "", fmt.Errorf("listing %s: %w", listPath, err) } pageItems, _ := extractItems(pageRaw) - for _, item := range pageItems { - if matchesName(item, name) { - if id := extractID(item); id != "" { - return id, nil - } - } + if id, err := firstMatch(pageItems, name); err != nil { + return "", err + } else if id != "" { + return id, nil } if len(pageItems) < pageSize { break @@ -121,6 +138,28 @@ func extractItems(raw json.RawMessage) ([]map[string]any, bool) { return nil, false } +// firstMatch scans items for entries whose name matches. Returns the ID of the +// first match, or an error when multiple items share the name within this page +// (ambiguous). Returns ("", nil) when no match is found. +func firstMatch(items []map[string]any, name string) (string, error) { + var matched []string + for _, item := range items { + if matchesName(item, name) { + if id := extractID(item); id != "" { + matched = append(matched, id) + } + } + } + switch len(matched) { + case 0: + return "", nil + case 1: + return matched[0], nil + default: + return "", fmt.Errorf("ambiguous match: %d items named %q; pass the positional ID to disambiguate", len(matched), name) + } +} + func matchesName(item map[string]any, name string) bool { for _, key := range []string{"name", "title", "displayName"} { if v, ok := item[key].(string); ok && v == name { diff --git a/internal/platform/resolve_generic_test.go b/internal/platform/resolve_generic_test.go index 5d97d16..55cd812 100644 --- a/internal/platform/resolve_generic_test.go +++ b/internal/platform/resolve_generic_test.go @@ -156,6 +156,62 @@ func TestResolveIDByName_NotFound(t *testing.T) { } } +// TestResolveIDByName_Ambiguous verifies that when two items share the same +// name on the same page, an error is returned rather than silently picking one. +func TestResolveIDByName_Ambiguous(t *testing.T) { + mux := http.NewServeMux() + path := "/api/device-groups/v1/tenant/" + resolveTestTenantID + "/device-groups" + mux.HandleFunc(path, func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{ + "results": []any{ + map[string]any{"id": "dg-computer", "name": "All Managed", "deviceType": "COMPUTER"}, + map[string]any{"id": "dg-mobile", "name": "All Managed", "deviceType": "MOBILE"}, + }, + "totalCount": 2, + }) + }) + client := newResolveTestClient(t, mux) + + _, err := ResolveIDByName(context.Background(), client, path, "All Managed") + if err == nil { + t.Fatal("expected ambiguous error, got nil") + } + if IsNotFound(err) { + t.Errorf("should not be ErrNotFound, got %v", err) + } +} + +// TestResolveIDByNameFiltered verifies that the filter expression is appended +// as a query param and narrows the result to the matching device type. +func TestResolveIDByNameFiltered(t *testing.T) { + mux := http.NewServeMux() + path := "/api/device-groups/v1/tenant/" + resolveTestTenantID + "/device-groups" + var capturedFilter string + mux.HandleFunc(path, func(w http.ResponseWriter, r *http.Request) { + capturedFilter = r.URL.Query().Get("filter") + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{ + "results": []any{ + map[string]any{"id": "dg-computer", "name": "All Managed", "deviceType": "COMPUTER"}, + }, + "totalCount": 1, + }) + }) + client := newResolveTestClient(t, mux) + + id, err := ResolveIDByNameFiltered(context.Background(), client, path, "All Managed", `deviceType=="COMPUTER"`) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if id != "dg-computer" { + t.Errorf("id = %q, want %q", id, "dg-computer") + } + if capturedFilter != `deviceType=="COMPUTER"` { + t.Errorf("filter = %q, want %q", capturedFilter, `deviceType=="COMPUTER"`) + } +} + // TestResolveIDByName_BareArray verifies matching against bare-array responses. func TestResolveIDByName_BareArray(t *testing.T) { mux := http.NewServeMux() diff --git a/internal/scope/types.go b/internal/scope/types.go index c62a928..da957ca 100644 --- a/internal/scope/types.go +++ b/internal/scope/types.go @@ -163,7 +163,7 @@ type ScopeXML struct { JSSUserGroups ScopeItemSlice `xml:"jss_user_groups" json:"jss_user_groups"` Buildings ScopeItemSlice `xml:"buildings" json:"buildings"` Departments ScopeItemSlice `xml:"departments" json:"departments"` - Classes ScopeItemSlice `xml:"classes" json:"classes,omitempty"` + Classes ScopeItemSlice `xml:"classes" json:"classes"` LimitToUsers *LimitToUsersXML `xml:"limit_to_users,omitempty" json:"limit_to_users,omitempty"` Limitations *LimitationsXML `xml:"limitations,omitempty" json:"limitations,omitempty"` Exclusions *ExclusionsXML `xml:"exclusions,omitempty" json:"exclusions,omitempty"` diff --git a/scripts/lint-dead-code/scan.go b/scripts/lint-dead-code/scan.go index 0eaf31f..4606f8c 100644 --- a/scripts/lint-dead-code/scan.go +++ b/scripts/lint-dead-code/scan.go @@ -343,7 +343,7 @@ func extractAllowlist(expr ast.Expr) map[string]bool { continue } set := map[string]bool{} - for _, p := range strings.Split(strings.Trim(vLit.Value, `"`), ",") { + for p := range strings.SplitSeq(strings.Trim(vLit.Value, `"`), ",") { if s := strings.TrimSpace(p); s != "" { set[s] = true }