Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fetch installed apps from iPhone/iPad devices. #20733

Merged
merged 9 commits into from
Jul 28, 2024
Merged

Fetch installed apps from iPhone/iPad devices. #20733

merged 9 commits into from
Jul 28, 2024

Conversation

getvictor
Copy link
Member

@getvictor getvictor commented Jul 25, 2024

Part 2 of #19447

  • iOS and iPadOS user-installed apps are loaded into Fleet
  • Added an additional identifier into software_titles table to differentiate between iOS/iPadOS apps
  • Updated nano queue timestamp precision

Note: TestIntegrationsMDM/TestVPPApps fails when run as part of the suite, but passes standalone. I'd like to proceed with merging this PR, and figure out the issue next week.

Checklist for submitter

  • Changes file added for user-visible changes in changes/, orbit/changes/ or ee/fleetd-chrome/changes.
    See Changes files for more information.
  • Added support on fleet's osquery simulator cmd/osquery-perf for new osquery data ingestion features.
  • Added/updated tests
  • Manual QA for all new/changed functionality

Copy link

codecov bot commented Jul 25, 2024

Codecov Report

Attention: Patch coverage is 41.76707% with 145 lines in your changes missing coverage. Please review.

Project coverage is 37.30%. Comparing base (02b88e6) to head (199657f).
Report is 19 commits behind head on main.

Files Patch % Lines
cmd/osquery-perf/agent.go 0.00% 97 Missing ⚠️
server/service/apple_mdm.go 61.85% 25 Missing and 12 partials ⚠️
server/service/hosts.go 33.33% 4 Missing and 2 partials ⚠️
cmd/fleet/cron.go 0.00% 5 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main   #20733       +/-   ##
===========================================
- Coverage   55.24%   37.30%   -17.94%     
===========================================
  Files        1423     1423               
  Lines      133632   134363      +731     
  Branches     3218     3216        -2     
===========================================
- Hits        73823    50123    -23700     
- Misses      53794    79713    +25919     
+ Partials     6015     4527     -1488     
Flag Coverage Δ
backend 35.89% <41.76%> (-19.48%) ⬇️

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.

roperzh
roperzh previously approved these changes Jul 26, 2024
Copy link
Member

@roperzh roperzh left a comment

Choose a reason for hiding this comment

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

LGTM, very exciting feature!

a few minor things that are just personal opinion, feel free to merge.

strings map[string]string
}

// stats, model, *serverURL, *mdmSCEPChallenge, *mdmCheckInInterval
Copy link
Member

Choose a reason for hiding this comment

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

leftover?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I'll clean up in the next PR.

software = append(software, fleet.Software{
Name: truncateString(app["Name"], fleet.SoftwareNameMaxLength),
Version: truncateString(app["ShortVersion"], fleet.SoftwareVersionMaxLength),
BundleIdentifier: truncateString(app["Identifier"], fleet.SoftwareBundleIdentifierMaxLength),
Copy link
Member

Choose a reason for hiding this comment

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

at some point we should crate structs for the info we fetch from ipad/ios

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense. I'll do that next time. Was following the pattern that was already there.

@@ -1683,7 +1685,8 @@ AND COALESCE(s.bundle_identifier, '') = '';
updateSoftwareWithIdentifierStmt := `
UPDATE software s
JOIN software_titles st
ON s.bundle_identifier = st.bundle_identifier
ON s.bundle_identifier = st.bundle_identifier AND
IF(s.source IN ('ios_apps', 'ipados_apps'), s.source = st.source, 1)
Copy link
Member

Choose a reason for hiding this comment

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

very nice solve

@@ -500,6 +500,26 @@ func (c *TestAppleMDMClient) AcknowledgeDeviceInformation(udid, cmdUUID, deviceN
return c.sendAndDecodeCommandResponse(payload)
}

func (c *TestAppleMDMClient) AcknowledgeInstalledApplicationList(udid, cmdUUID string, software []fleet.Software) (*mdm.Command, error) {
Copy link
Member

Choose a reason for hiding this comment

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

my personal preference would be to keep the methods of the mdmtest client reduced to only the protocol operations, it's a bit more verbose for the caller, so I understand the motivation for this.

@getvictor getvictor merged commit 671fc62 into main Jul 28, 2024
17 checks passed
@getvictor getvictor deleted the 20468-ios-apps branch July 28, 2024 14:17
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.

None yet

3 participants