SetOrUpdateMunkiInfo Method can't revert a deleted record back to be not deleted

[limingao666]

Fleet version: 4.32.0

🧑‍💻  Expected behavior

If munki information is shown as long as the host is reporting it.

💥  Actual behavior

Munki information that was marked as deleted at some point in time is not shown anymore, even if the host reports munki info again.

More info

In SetOrUpdateMunkiInfo, we set host_munki_info.deleted_at = NOW() if we don't receive a version:

https://github.com/fleetdm/fleet/blob/b371b9cebc71172c60ef43693c94b94dafcf2a23/server/datastore/mysql/hosts.go#L2469-L2477

However, if we receive a version later on, we don't set back the value to NULL:

https://github.com/fleetdm/fleet/blob/b371b9cebc71172c60ef43693c94b94dafcf2a23/server/datastore/mysql/hosts.go#L2478-L2483

This affects some of the queries we do that filter by deleted_at, for example:

https://github.com/fleetdm/fleet/blob/b371b9cebc71172c60ef43693c94b94dafcf2a23/server/datastore/mysql/hosts.go#L2854

Reproduction steps

1. Install munki on the host
2. Refetch the host
3. See in the UI that you get munki info
4. Remove munki from the host
5. Refetch the host
6. Verify that you don't get munki info in the UI anymore
7. Install munki again
8. Refetch the host

At this point, you should see munki info again, but you won't see it.

[roperzh]

hey @limingao666 thank you for raising this! I updated the issue description with more information, and I agree that this is a bug.

Please feel free to submit a PR!

[sharon-fdm]

@xpkoala, Please reproduce and tell me what you think.

[xpkoala]

@sharon-fdm It appears that @roperzh has identified the issue in the code and this issue can be fast tracked for prioritization. I appear to have reproduced the issue by removing and reinstalling munki on a Mac host, but I am not 100% confident in my reproduction steps as I encountered some errors when attempting to remove the old version of Munki.

@roperzh Is this something that can be encountered by simply removing munki via the munki uninstall scripts and then reinstalling the manager? Or is this an edge case that we don't expect to be encountered from these types of actions? These are the commands I ran for removing the app and associated pieces.

[roperzh]

@xpkoala yes exactly, full repro steps (not tested) would be something like:

1. Install munki on the host
2. Refetch the host
3. See in the UI that you get munki info
4. Remove munki from the host
5. Refetch the host
6. Verify that you don't get munki info in the UI anymore
7. Install munki again
8. Refetch the host

At this point, you should see munki info again, but you won't see it. It's an edge case so I'm not sure about the priority of this issue.

[xpkoala]

The above steps are what I ran through and reproduced. I'll re-tag this one for prioritization. Thank you!

[RachelElysia]

@lucasmrod @juan-fdz-hawa putting this on your radar that we will estimate this on Wednesday. Let me know if there's anything more that should be clarified prior to estimation. 🌼

[RachelElysia]

@lucasmrod @juan-fdz-hawa "Fix should be straight forward, but the setup and testing might take longer"

Estimate: 2-3 pts (1pt for the fix + 1-2pt for setup and testing)

Potential engineers to assign: Any Go engineers @lucasmrod @juan-fdz-hawa

[lukeheath]

@sharon-fdm This bug is aging out. This seems unnecessary since @roperzh already identified the fix.

[sharon-fdm]

@lukeheath I'm not sure I understand. Was this fixed in another ticket/PR?

[lukeheath]

@sharon-fdm - Roberto updated the ticket to include reproduction steps and details what the issue is in the code. Should be straightforward to fix, although as noted it does require installing Munki.

[ireedy]

Bug has aged out. Moved back to drafting

[ireedy]

Bug has aged out. Moved back to drafting

[noahtalerman]

@sharon-fdm do you think Endpoint ops can estimate and take this bug in when we bring in more bugs this sprint? It's an old bug.

[sharon-fdm]

@noahtalerman from a quick look at the history it looks like we can take it. Put it high in the backlog.

[lukeheath]

Bug has aged out. Moving back to product drafting.

[fleet-release]

Munki info reborn, Lives in Fleet's cloud city high, Data lost restored.