-
Notifications
You must be signed in to change notification settings - Fork 427
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
address PR feedback for FileVault improvements #20935
Conversation
feedback left by @mna and @gillespi314 in #20842 related to #13157
if orbitClient.GetServerCapabilities().Has(fleet.CapabilityEscrowBuddy) { | ||
orbitClient.RegisterConfigReceiver(update.NewEscrowBuddyRunner(updateRunner, 5*time.Minute)) | ||
} else { | ||
orbitClient.RegisterConfigReceiver(update.ApplyDiskEncryptionRunnerMiddleware()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Someday we should rename ApplyDiskEncryptionRunnerMiddleware
to make clear that it is macOS-only. Feel free to leave it as-is though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, probably all of those, the majority still are named middlewares.
@@ -870,11 +870,6 @@ func main() { | |||
orbitClient.RegisterConfigReceiver(update.ApplyNudgeConfigReceiverMiddleware(update.NudgeConfigFetcherOptions{ | |||
UpdateRunner: updateRunner, RootDir: c.String("root-dir"), Interval: nudgeLaunchInterval, | |||
})) | |||
if orbitClient.GetServerCapabilities().Has(fleet.CapabilityEscrowBuddy) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the fix for the issue Gabe found? Is it that we haven't initiated the capabilities checker yet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's right, sorry for not being clear about it.
tests are failing until an endpoint ops feature is merged. Merging |
feedback left by @mna and @gillespi314 in #20842
also fixes a bug found by @PezHub #13157 (comment)
related to #13157
Checklist for submitter
If some of the following don't apply, delete the relevant line.