Skip to content

Don't delete Android agent when transferring teams. - #37517

Merged
getvictor merged 2 commits into
mainfrom
victor/37440-team-transfer
Dec 20, 2025
Merged

Don't delete Android agent when transferring teams.#37517
getvictor merged 2 commits into
mainfrom
victor/37440-team-transfer

Conversation

@getvictor

@getvictor getvictor commented Dec 18, 2025

Copy link
Copy Markdown
Member

Related issue: Resolves #37440

Note, from manual testing, when moving host to a new team, it does NOT get the certs for the new team. We could fix this as part of this fix or in a separate PR.

Testing

  • Added/updated automated tests
  • QA'd all new/changed functionality manually

For unreleased bug fixes in a release candidate, one of:

  • Confirmed that the fix is not expected to adversely impact load test results

Summary by CodeRabbit

  • New Features
    • Added Fleet Agent integration for Android MDM devices
    • Fleet Agent policies are now automatically constructed and applied to managed Android devices
    • Fleet Agent configuration includes server URL, enrollment secrets, and certificate templates

✏️ Tip: You can customize this high-level summary in your review settings.

@getvictor

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Dec 18, 2025

Copy link
Copy Markdown
Contributor
✅ Actions performed

Full review triggered.

@coderabbitai

coderabbitai Bot commented Dec 18, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Introduces helper functions and service methods for constructing Fleet Agent application policies in Android MDM. Factors out package info retrieval, policy construction, and per-host configuration assembly into reusable components. Refactors existing logic in AddFleetAgentToAndroidPolicy and ensures Fleet Agent policies are preserved during bulk policy updates by explicitly appending them to application policy lists.

Changes

Cohort / File(s) Change Summary
Android MDM Interface and Service Methods
server/mdm/android/service.go
Adds new interface method BuildFleetAgentApplicationPolicy(ctx, hostUUID) to the Service interface.
Android MDM Service Implementation
server/mdm/android/service/service.go
Introduces helper functions: getFleetAgentPackageInfo() (reads env vars), buildFleetAgentAppPolicy() (constructs policy), buildAgentManagedConfig() (builds per-host config). Adds public method BuildFleetAgentApplicationPolicy(). Refactors AddFleetAgentToAndroidPolicy to use new helpers.
Software Worker Integration
server/worker/software_worker.go
Modifies bulkSetAndroidAppsAvailableForHosts to build and append Fleet Agent policy to the appPolicies list; logs errors but continues without interrupting flow.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Areas requiring attention:
    • Verify the refactored AddFleetAgentToAndroidPolicy logic correctly handles cases where package info is not configured (should be a no-op)
    • Confirm error handling in buildAgentManagedConfig properly wraps and propagates configuration errors
    • Validate the integration in bulkSetAndroidAppsAvailableForHosts correctly appends the Fleet Agent policy and that error logging doesn't mask critical failures
    • Ensure environment variable parsing in getFleetAgentPackageInfo handles edge cases (missing SHA256 when package is set, etc.)

Possibly related PRs

  • Install Fleet android agent on device enrollment. #36050: Introduced AddFleetAgentToAndroidPolicy and AgentManagedConfiguration structures; this PR refactors and extends that foundation with policy construction helpers and per-host configuration building.
  • Bootstrapping Android app #36233: Also modifies Android Fleet Agent provisioning and ApplicationPolicy construction using package name, SHA256 fingerprint, and certificate configuration; directly related to the policy construction logic in this PR.

Suggested reviewers

  • georgekarrv
  • dantecatalfamo
  • jahzielv

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is incomplete, missing several required sections from the template including changes files, database migrations, and specific testing details. Add changes file entry, confirm database migration considerations, expand testing section with specific test details, and verify all checklist items align with actual changes.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: preventing deletion of Android agent during team transfers, which aligns with the core issue being addressed.
Linked Issues check ✅ Passed The code changes implement the solution to prevent Fleet agent deletion during team transfers by refactoring policy construction, adding helper methods to build and preserve agent policies, and ensuring the agent policy is appended to the list during team transfers.
Out of Scope Changes check ✅ Passed All changes are scoped to Android MDM and directly address the issue: adding Fleet agent policy helpers in the service layer and integrating them into the software worker's bulk application policy setting.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch victor/37440-team-transfer

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecov Bot commented Dec 18, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 46.98795% with 44 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.92%. Comparing base (24cd9ce) to head (ad65d6b).
⚠️ Report is 47 commits behind head on main.

Files with missing lines Patch % Lines
server/mdm/android/service/service.go 47.43% 36 Missing and 5 partials ⚠️
server/worker/software_worker.go 40.00% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main   #37517    +/-   ##
========================================
  Coverage   65.92%   65.92%            
========================================
  Files        2357     2357            
  Lines      186680   186857   +177     
  Branches     7853     7853            
========================================
+ Hits       123076   123194   +118     
- Misses      52359    52407    +48     
- Partials    11245    11256    +11     
Flag Coverage Δ
backend 67.74% <46.98%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@getvictor
getvictor marked this pull request as ready for review December 19, 2025 18:45
@getvictor
getvictor requested a review from a team as a code owner December 19, 2025 18:46
@getvictor
getvictor merged commit 5000723 into main Dec 20, 2025
45 checks passed
@getvictor
getvictor deleted the victor/37440-team-transfer branch December 20, 2025 00:05
getvictor added a commit that referenced this pull request Dec 20, 2025
<!-- Add the related story/sub-task/bug number, like Resolves #123, or
remove if NA -->
**Related issue:** Resolves #37440

Note, from manual testing, when moving host to a new team, it does NOT
get the certs for the new team. We could fix this as part of this fix or
in a separate PR.

## Testing

- [x] Added/updated automated tests
- [x] QA'd all new/changed functionality manually

For unreleased bug fixes in a release candidate, one of:

- [x] Confirmed that the fix is not expected to adversely impact load
test results

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
  * Added Fleet Agent integration for Android MDM devices
* Fleet Agent policies are now automatically constructed and applied to
managed Android devices
* Fleet Agent configuration includes server URL, enrollment secrets, and
certificate templates

<sub>✏️ Tip: You can customize this high-level summary in your review
settings.</sub>

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

(cherry picked from commit 5000723)
getvictor added a commit that referenced this pull request Dec 20, 2025
<!-- Add the related story/sub-task/bug number, like Resolves #123, or
remove if NA -->
**Related issue:** Resolves #37440

Note, from manual testing, when moving host to a new team, it does NOT
get the certs for the new team. We could fix this as part of this fix or
in a separate PR.

## Testing

- [x] Added/updated automated tests
- [x] QA'd all new/changed functionality manually

For unreleased bug fixes in a release candidate, one of:

- [x] Confirmed that the fix is not expected to adversely impact load
test results

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
  * Added Fleet Agent integration for Android MDM devices
* Fleet Agent policies are now automatically constructed and applied to
managed Android devices
* Fleet Agent configuration includes server URL, enrollment secrets, and
certificate templates

<sub>✏️ Tip: You can customize this high-level summary in your review
settings.</sub>

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

(cherry picked from commit 5000723)
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.

Fleet agent app gets deleted when an Android host is transferred from one team to another

3 participants