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

syncservice: implementation and migration #775

Merged
merged 5 commits into from
Apr 27, 2022
Merged

Conversation

tburgin
Copy link
Contributor

@tburgin tburgin commented Mar 29, 2022

Santa's syncing architecture is currently based around a user centric command line application (santactl sync). Over the years we have daemon-ized portions of santactl to support features like push notifications and bundle voting. This works but added significant complexity to santactl. This pull request attempts to right this past wrong by splitting out all the syncing code into a separate service.

The syncservice is a new binary/process that is launched on demand as a MachService. If a sync server is configured, syncservice will start up and perform periodic syncs, handle push notifications and handle event and bundle uploads from santad. Some highlights:

  • The syncing code have been moved from santactl to syncservice. The bulk of that work was completed in santasyncservice: move sync code to the santasyncservice dir #696 and is now finished in this pull.
  • syncservice is started by launchd, as opposed to the previous approach of santad execing santactl sync.
  • santactl sync now communicates directly with the syncservice to sync.
  • All syncing across the system now takes place in the syncservice. The syncservice ensures concurrent syncs do not happen.

Fixes #653

@tburgin tburgin added this to the 2022.4 Release milestone Mar 29, 2022
@pmarkowsky pmarkowsky added the sync service Issues related to the sync service / protocol label Mar 30, 2022
@pmarkowsky
Copy link
Contributor

Still looking through this.

Annotated High level flow through the syncservice after this PR is.

sequenceDiagram
   santactl ->> santasyncservice: syncWithLogListener
   Note over santactl,santasyncservice: Service is spun up by launchd if needed
   alt is clean sync
      santasyncservice ->> santad: "setSyncCleanRequired:YES"
   else 
   end
   santasyncservice ->> syncserver: preflight
   santasyncservice --> santactl: "didReceiveLog (messages for preflight)"
   syncserver -->> santasyncservice: preflight response
   santasyncservice --> santactl: "didReceiveLog (messages for preflight response)"
   santasyncservice ->> syncserver: eventUpload
   syncserver -->> santasyncservice: eventUploadResponse (200)
   santasyncservice ->> syncserver: ruleDownload
   santasyncservice --> santactl: "didReceiveLog (messages for ruleDownload )"
   syncserver --> santasyncservice: ruleDownload response (200)
   santasyncservice ->> santad: databaseRuleAddRules
   santasyncservice ->> santad: setRuleSyncLastSuccess
   santasyncservice ->> santad: postRuleSyncNotificationWithCustomMessage
   santasyncservice --> santactl: "didReceiveLog (messages for ruleDownload response)"
   santasyncservice ->> syncserver: postFlight request
   santasyncservice -->> santactl: "didReceiveLog (messages for postFlight)"
   syncserver -->> santasyncservice: postFlight response
   santasyncservice -->> santactl: "didReceiveLog (messages for postFlight response)"
   santasyncservice -->> santactl: "didReceiveLog (final messages)"
   
Loading

@russellhancox
Copy link
Contributor

Still looking through this.

Annotated High level flow through the syncservice after this PR is.

santad is triggering the sync service to start in SNTApplication (if a sync server URL is set), this is needed to have push notifications to work and (if I'm not mistaken) perform syncs on interval when needed.

Source/common/SNTLogging.m Outdated Show resolved Hide resolved
Source/common/SNTLogging.m Outdated Show resolved Hide resolved
Source/common/SNTLogging.h Outdated Show resolved Hide resolved
Source/common/SNTXPCSyncServiceInterface.h Outdated Show resolved Hide resolved
Source/santactl/Commands/SNTCommandSync.m Outdated Show resolved Hide resolved
Source/santasyncservice/BUILD Show resolved Hide resolved
Source/santactl/Commands/SNTCommandSync.m Show resolved Hide resolved
Source/common/SNTLogging.h Outdated Show resolved Hide resolved
Source/santasyncservice/SNTSyncService.m Outdated Show resolved Hide resolved
Source/santasyncservice/SNTSyncService.m Outdated Show resolved Hide resolved
@tburgin
Copy link
Contributor Author

tburgin commented Apr 22, 2022

PTAL

Thanks @mlw for the comment about dispatching syncs onto a queue. That simplified things a lot.

Some additional refactoring was needed due to the serialization. Two new classes SNTPushNotifications and SNTPushNotificationsTracker were created to isolate the push notifications logic as much as possible. It looks like there is a lot of new code, but it is mainly a forklift.

There are two non-blocking TODOs. Copying them here for visibility.

From Source/santasyncservice/SNTSyncLogging.h

// TODO(bur): SLOGD() is temporarily set to LOG_LEVEL_INFO. Once santactl sync supports the
// --debug flag, move this back to LOG_LEVEL_DEBUG. These debug logs are helpful when
// troubleshooting sync issues with users, so let's opt to always log them for now.

From Source/santasyncservice/SNTPushNotificationsTracker.m

// TODO(bur): Add better guaranties for displaying notifications. This will involve checking the
// rules.db to see if the rule associated with the notification has been applied.

Source/common/SNTCommonEnums.h Outdated Show resolved Hide resolved
Source/common/SNTLogging.m Show resolved Hide resolved
Source/common/SNTLogging.m Outdated Show resolved Hide resolved
Source/common/SNTXPCSyncServiceInterface.h Outdated Show resolved Hide resolved
Source/santasyncservice/SNTPushNotifications.m Outdated Show resolved Hide resolved
Source/santasyncservice/SNTPushNotifications.m Outdated Show resolved Hide resolved
Source/santasyncservice/SNTSyncLogging.m Outdated Show resolved Hide resolved
@tburgin tburgin merged commit b5ebe12 into google:main Apr 27, 2022
@tburgin tburgin deleted the malevolence branch April 27, 2022 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sync service Issues related to the sync service / protocol
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Running sync clean in parallel with multiple pages of rules can corrupt rule database
4 participants