Fix code default profile - #44601
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
There was a problem hiding this comment.
Pull request overview
Updates Fleet’s in-code default Apple DEP enrollment profile to match the intended “default profile” configuration (as used when no custom Setup Assistant profile is provided).
Changes:
- Simplifies the default DEP profile fields to a smaller, stricter default (supervised + non-removable).
- Clears the previous default skip list and other previously hard-coded defaults in favor of a minimal baseline.
- Adds an inline note to keep the fleetctl “new” template in sync with this default.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
WalkthroughThe Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/mdm/apple/apple_mdm.go`:
- Line 110: The struct literal sets SkipSetupItems to an empty slice
([]string{}) which will be marshaled away due to the
`json:"skip_setup_items,omitempty"` tag; change the initialization in
apple_mdm.go to use nil for SkipSetupItems (i.e., omit the field or set it to
nil) so it round-trips as nil and matches the expected in-memory default used by
the equality check in server/service/integration_mdm_dep_test.go; if you
actually need an explicit empty array instead, adjust the JSON tag on
godep.Profile.SkipSetupItems instead of using []string{} here.
🪄 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: 3e5031d7-b90b-44b7-8bd6-0e2ae34f7801
📒 Files selected for processing (1)
server/mdm/apple/apple_mdm.go
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #44601 +/- ##
==========================================
+ Coverage 66.68% 66.70% +0.01%
==========================================
Files 2645 2649 +4
Lines 212833 213274 +441
Branches 9610 9610
==========================================
+ Hits 141928 142262 +334
- Misses 58002 58064 +62
- Partials 12903 12948 +45
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Removed the empty slice, and I think it's fine to have that in the .json, as it will be unmarshalled into nil, since it's empty. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/mdm/apple/apple_mdm.go (1)
105-105: ⚡ Quick winMake the sync note actionable with an exact template path.
This reminder is helpful; please include the concrete
fleetctl newtemplate file path in the comment so future updates don’t rely on tribal knowledge.♻️ Suggested tweak
- // If this definition change, make sure to update the fleetctl new template file + // If this definition changes, update <exact fleetctl template filepath> in the same PR.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/mdm/apple/apple_mdm.go` at line 105, Update the inline comment that currently reads "// If this definition change, make sure to update the fleetctl new template file" in apple_mdm.go so it includes the exact repository-relative path to the corresponding fleetctl "new" template (replace the vague reminder with a concrete path to the template file used by fleetctl new), e.g., change the comment on that line to reference the precise template file location so future edits know exactly which template to update.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@server/mdm/apple/apple_mdm.go`:
- Line 105: Update the inline comment that currently reads "// If this
definition change, make sure to update the fleetctl new template file" in
apple_mdm.go so it includes the exact repository-relative path to the
corresponding fleetctl "new" template (replace the vague reminder with a
concrete path to the template file used by fleetctl new), e.g., change the
comment on that line to reference the precise template file location so future
edits know exactly which template to update.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d967f0f4-2c9d-4fe8-abba-1db49976dafa
📒 Files selected for processing (1)
server/mdm/apple/apple_mdm.go
Only question about removing the empty slice does that cause it to send null to Apple? or empty array? Can you tell? I wonder if the Apple API is OK with null |
Summary by CodeRabbit