Skip to content

feat: add ManagedDevice resource type + syncer#24

Draft
c1-squire-dev[bot] wants to merge 4 commits into
mainfrom
paul.querna/managed-device-syncer
Draft

feat: add ManagedDevice resource type + syncer#24
c1-squire-dev[bot] wants to merge 4 commits into
mainfrom
paul.querna/managed-device-syncer

Conversation

@c1-squire-dev

@c1-squire-dev c1-squire-dev Bot commented Jul 1, 2026

Copy link
Copy Markdown

Summary

Adds a net-new ManagedDevice resource type + ResourceSyncer to baton-jamf, emitting the new ManagedDeviceTrait. baton-jamf was identity-only before this; this is the connector's first device inventory sync.

The syncer walks two Jamf Pro endpoints, paginated via the SDK pagination.Bag idiom (one page per List call, never unrolled):

  • GET /api/v1/computers-inventory — section-gated: GENERAL, HARDWARE, OPERATING_SYSTEM, USER_AND_LOCATION, DISK_ENCRYPTION, SECURITY
  • GET /api/v2/mobile-devices

managedDevice is a pure asset: Entitlements and Grants return empty; the resource type is annotated SkipEntitlementsAndGrants.

Field mapping (ManagedDeviceTrait)

Trait field Source
serial hardware.serialNumber
udid udid
device_type LAPTOP/DESKTOP from Apple model identifier (book ⇒ laptop); MOBILE/TABLET for mobile (iPad ⇒ tablet)
model / vendor hardware.model (fallback modelIdentifier) / hardware.make
os DeviceOS{type,name,version,build} from operatingSystem.*; type mapped macOS/iOS/iPadOS
assigned_user resolves userAndLocation.username/email to the synced Jamf user resource and cross-links its ResourceId; unresolved owners stashed as unresolved_owner profile entry
is_encrypted tri-state from diskEncryption.bootPartitionEncryptionDetails.partitionFileVault2State
is_supervised general.supervised
management_state MANAGED when general.mdmCapable.capable && general.remoteManagement.managed, else UNSPECIFIED
enrolled_at general.lastEnrolledDate
profile site, building/department/room/position, SECURITY posture (activationLock/recoveryLock/firewall/SIP/gatekeeper)

Deliberately not set (no faithful Jamf source): hardware_hash, compliance (Jamf has no first-class compliance verdict — left UNSPECIFIED rather than fabricated), and any last-seen field (omitted per RFC-C v1 — lastContactTime/reportDate intentionally not mapped). Fields absent from a given payload are left unset rather than guessed.

Error discipline

A transient/auth API failure returns a non-nil error and never an empty resource list, so a failed page cannot silently prune the device inventory.

Dependency

Depends on baton-sdk#995 (ManagedDeviceTrait + TRAIT_MANAGED_DEVICE), pinned at commit 87bfe5d1a2a37c22fd69e534b479d525d9c3e50f. Re-pin go.mod to the released baton-sdk tag once #995 merges, then go mod tidy && go mod vendor.

Verification

go build ./..., go vet ./..., go test ./... (11 pass, incl. new device-mapping unit tests), golangci-lint run, and gofmt all clean. baton_capabilities.json regenerated via the connector capabilities command.

Draft — do not merge. Held for human review + the baton-sdk#995 merge/re-pin.

Adds a net-new ManagedDevice resource type that emits the ManagedDeviceTrait
introduced in baton-sdk. The syncer walks the Jamf Pro computers-inventory
endpoint (section-gated: GENERAL, HARDWARE, OPERATING_SYSTEM,
USER_AND_LOCATION, DISK_ENCRYPTION, SECURITY) and the v2 mobile-devices
endpoint, paginating via the SDK pagination.Bag idiom. Devices are pure
assets: Entitlements and Grants return empty.

Field mapping onto ManagedDeviceTrait: serial, udid, device_type
(LAPTOP/DESKTOP for computers, MOBILE/TABLET for mobile), model, vendor,
os (DeviceOS type/name/version/build), is_encrypted (FileVault2 tri-state),
is_supervised, management_state (MANAGED when MDM-capable + remotely
managed), enrolled_at, and a long-tail profile (site, building/department,
security posture). assigned_user resolves the Jamf owner
username/email to the synced Jamf user resource and cross-links it;
unresolved owners are stashed as an unresolved_owner profile entry.
compliance is left UNSPECIFIED (Jamf has no first-class verdict) and no
last-seen field is emitted (per RFC-C v1).

Depends on baton-sdk PR #995 (ManagedDeviceTrait + TRAIT_MANAGED_DEVICE),
pinned at commit 87bfe5d1a2a37c22fd69e534b479d525d9c3e50f; re-pin to the
released tag once #995 merges.

Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
Comment on lines +73 to +98
case devicePhaseComputer:
resp, err := d.client.GetComputersInventory(ctx, page, pageSize, jamf.ComputerInventorySections)
if err != nil {
return nil, nil, fmt.Errorf("jamf-connector: failed to list computers inventory: %w", err)
}
for i := range resp.Results {
r, err := computerResource(&resp.Results[i], userIndex, parentId)
if err != nil {
return nil, nil, fmt.Errorf("jamf-connector: failed to build computer resource: %w", err)
}
resources = append(resources, r)
}
if hasMorePages(page, pageSize, resp.TotalCount, len(resp.Results)) {
if err := bag.Next(strconv.Itoa(page + 1)); err != nil {
return nil, nil, err
}
} else {
// Computers exhausted; advance to the mobile-device phase.
bag.Pop()
bag.Push(pagination.PageState{ResourceTypeID: devicePhaseMobile, Token: "0"})
}

case devicePhaseMobile:
resp, err := d.client.GetMobileDevices(ctx, page, pageSize)
if err != nil {
return nil, nil, fmt.Errorf("jamf-connector: failed to list mobile devices: %w", err)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion (medium confidence): The two device endpoints (/api/v1/computers-inventory, /api/v2/mobile-devices) require the Jamf API privileges "Read Computers" / "Read Mobile Devices". Existing installs whose API role lacks these will get a 403, which GetComputersInventory/GetMobileDevices surface as an error, failing the entire connector sync (not just devices) after this deploys. Since managedDevice is a brand-new resource type with nothing to prune, consider degrading gracefully on 403/PermissionDenied (warn + return an empty page) so a missing scope doesn't break previously-working syncs. Hard-failing on transient/other errors (to avoid silent pruning) is still the right call.

Comment thread go.mod Outdated

require (
github.com/conductorone/baton-sdk v0.8.14
github.com/conductorone/baton-sdk v0.16.1-0.20260701222205-87bfe5d1a2a3

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: baton-sdk is pinned to an unreleased pseudo-version (v0.16.1-0.20260701222205-87bfe5d1a2a3) tracking the unmerged baton-sdk#995. Re-pin to the released tag before this merges (as the PR description notes), otherwise the connector depends on a mutable, unreleased commit for ManagedDeviceTrait / TRAIT_MANAGED_DEVICE.

return d.userIndex, nil
}

users, err := d.client.GetUsers(ctx)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion (low confidence): GetUsers performs an N+1 fetch (a getUserDetails call per user) that the user syncer already ran this sync; building the owner index here repeats all of it. For large tenants this roughly doubles user-detail API calls. Acceptable given it's cached once per sync, but worth noting if device syncs get slow.

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Connector PR Review: feat: add ManagedDevice resource type + syncer

Blocking Issues: 0 | Suggestions: 3 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base cca44f7.
Review mode: incremental since b6cb470
View review run

Review Summary

Scanned the full PR diff (non-vendored: managedDevice.go, its test, connector.go, device_client.go, device_models.go, docs, go.mod/go.sum) for security and correctness. The new commit refactors assignee handling: the identity previously stashed in the ManagedDeviceTrait profile (removed in the re-pinned baton-sdk) is now carried in an in-memory deviceOwners map populated during List and read in Entitlements/Grants; the SDK bump and test updates match. No security issues. One new correctness concern about the in-memory map not surviving a resumed sync; the two prior findings (pagination early-termination, unreleased SDK pseudo-version) remain open.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

  • pkg/connector/managedDevice.go:515-534 — assignee identity now lives only in the in-memory deviceOwners map (populated in List); a checkpointed/resumed sync that reaches the grants phase without re-running List would find it empty and silently drop all device-to-user grants. (medium confidence)
  • pkg/connector/managedDevice.go:344-352 — (previously raised, still open) hasMorePages derives progress from the requested pageSize; if Jamf caps page-size below the SDK-supplied size, pages return fewer records and the sync can terminate early, dropping devices.
  • go.mod:6 — (previously raised, still open) baton-sdk re-pinned to a newer unreleased pseudo-version (v0.16.1-0.20260703004916-3daeab170e99). Acknowledged in the PR body; re-pin to the released tag before merging (PR is correctly marked draft / do-not-merge).
Prompt for AI agents
Verify each finding against the current code and only fix it if needed.

## Suggestions

In pkg/connector/managedDevice.go:
- Around line 515-534: The deviceOwners map is populated only in List and read in deviceAssignee (used by Entitlements/Grants). Because it is in-memory only, a checkpointed sync that resumes into the grants phase without re-running List will read an empty map and silently emit no device-to-user grants. Make the assignee resilient to resume: either re-derive it in Grants (fetch the device by resource id like getUserIndex re-fetches users), or persist the assignee on the resource so it survives across process restarts.
- Around line 344-352: hasMorePages(page, pageSize, totalCount, gotThisPage) uses the requested pageSize to project progress. If Jamf caps page-size below the SDK-supplied value, each page returns fewer records than pageSize, so the projected offset overshoots totalCount and the sync stops early, dropping devices. Track cumulative records returned, or clamp pageSize to the Jamf maximum page size.
- Around line 6 of go.mod: baton-sdk is pinned to an unreleased pseudo-version. Re-pin to the released baton-sdk tag once the upstream ManagedDeviceTrait change is released, then run go mod tidy and go mod vendor.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues found.

baton-sdk removed assigned_user from ManagedDeviceTrait. Re-pin to the
field-removed head and emit the device->user link as an "assigned"
assignment entitlement + grant instead of a trait field.

The device syncer already reads the assignee (computers:
USER_AND_LOCATION.username/email; mobile: username). A synced Jamf user
is granted directly by ResourceId; an unsynced assignee produces a grant
carrying an ExternalResourceMatch(TRAIT_USER) so the platform can bind it
to a directory identity. Devices with no assignee emit neither entitlement
nor grant.

Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
Comment thread baton_capabilities.json Outdated
Comment on lines +24 to +28
"annotations": [
{
"@type": "type.googleapis.com/c1.connector.v2.SkipEntitlementsAndGrants"
}
]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: This generated file is now stale. This commit removed SkipEntitlementsAndGrants from the managedDevice resource type in connector.go and added a real Entitlements/Grants implementation (the "assigned" entitlement + device→user grants), but the capabilities descriptor still advertises SkipEntitlementsAndGrants here. Regenerate via the connector capabilities command so the published metadata matches the runtime behavior.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues found.

The ManagedDevice resource type no longer skips entitlements/grants, so
the committed baton_capabilities.json no longer carries the
SkipEntitlementsAndGrants annotation. Regenerated via
./connector capabilities to match the binary output.

Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
Comment on lines +364 to +372
func hasMorePages(page, pageSize, totalCount, gotThisPage int) bool {
if gotThisPage == 0 {
return false
}
if totalCount > 0 {
return (page+1)*pageSize < totalCount
}
return gotThisPage >= pageSize
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: hasMorePages computes progress from the requested pageSize ((page+1)*pageSize < totalCount). If Jamf caps page-size below the value the SDK supplies via attrs.PageToken.Size, each page returns fewer records than pageSize, so this arithmetic overshoots totalCount and terminates early — silently dropping devices from the inventory. Consider tracking cumulative records returned, or clamp the requested page size to Jamf's documented maximum. (medium confidence)

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues found.

… profile removed)

The per-trait profile Struct was removed from ManagedDeviceTrait. The device->user
assignee was previously stashed in that profile to reach the Entitlements/Grants
phases; it is now recorded in an in-memory map during List and read back by
Entitlements/Grants. Device long-tail extras that had no first-class trait field
are dropped. Re-pinned baton-sdk to the profile-removed revision.

Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
Comment on lines +515 to +534
func (d *managedDeviceResourceType) recordDeviceOwner(resource *v2.Resource, username, email string) {
if username == "" && email == "" {
return
}
d.mu.Lock()
defer d.mu.Unlock()
if d.deviceOwners == nil {
d.deviceOwners = make(map[string]deviceOwner)
}
d.deviceOwners[resource.GetId().GetResource()] = deviceOwner{username: username, email: email}
}

// deviceAssignee returns the assignee identity (username, email) recorded for a
// device during List. Empty strings mean the device reports no owner.
func (d *managedDeviceResourceType) deviceAssignee(resource *v2.Resource) (string, string) {
d.mu.Lock()
defer d.mu.Unlock()
o := d.deviceOwners[resource.GetId().GetResource()]
return o.username, o.email
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: The assignee identity now lives only in the in-memory deviceOwners map, populated exclusively during List. Unlike the previous profile-on-resource approach (which persisted in the c1z), this map does not survive a checkpointed/resumed sync. If a sync is interrupted after the resource-listing step completes but before the grants step finishes, on resume List is not re-run, deviceOwners is empty, and deviceAssignee returns ""—so Entitlements/Grants silently emit nothing and every device→user assignment is dropped. Consider re-deriving the assignee in Grants (as getUserIndex already re-fetches users) or persisting it on the resource so it is resilient to resume. (medium confidence)

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant