-
Notifications
You must be signed in to change notification settings - Fork 100
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
add database reset history to uninstall and add uninstall history checkup #1639
Conversation
|
||
for _, uninstallRecord := range resetRecords { | ||
resetTimeKey := time.Unix(uninstallRecord.ResetTimestamp, 0) | ||
hc.data[resetTimeKey.Format(time.RFC3339)] = map[string]any{ |
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.
This is fine, but feels like a bit of overkill. The time sorted array was probably fine
return nil | ||
} | ||
|
||
resetRecordsRaw, err := hc.k.PersistentHostDataStore().Get(agent.HostDataKeyResetRecords) |
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.
Consider moving this to something like agent.GetResetRecords(ctx context.Context, k types.Knapsack)
maybe.
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.
I will go ahead and do that, it should clean this all up a bit. thank you!
…s and clean up exported key
383141f
ee/agent/reset.go
Outdated
@@ -17,7 +17,7 @@ import ( | |||
"github.com/kolide/launcher/pkg/traces" | |||
) | |||
|
|||
type dbResetRecord struct { | |||
type DBResetRecord struct { |
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.
I'm not totally sure why this is exported
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.
oh I might be able to un-export this one too after moving to using agent.GetResetRecords
, will give that a try
Co-authored-by: seph <seph@kolide.co>
Co-authored-by: seph <seph@kolide.co>
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.
Let's try!
Co-authored-by: seph <seph@kolide.co>
These changes rework the remote uninstall path to utilize the database reset record functionality (added to support remediation for hardware changes). Now, an uninstallation will first take a record of the previous database before resetting.
This also adds an Uninstall History checkup, which will read from the reset records key to report if any previous uninstallations/resets have been recorded.