fix: clear auto install policy statuses for vpp app scope changes #26121
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #26121 +/- ##
=======================================
Coverage 63.59% 63.60%
=======================================
Files 1631 1631
Lines 156402 156486 +84
Branches 4037 4037
=======================================
+ Hits 99471 99535 +64
- Misses 49093 49110 +17
- Partials 7838 7841 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
In addition to the comments here, seems like we're missing the policy status clear side effect for bulk VPP app actions (for GitOps). Given that the code paths for labels are basically the same for package-based installers and VPP apps, thinking we're missing the side effect for both, and the code for fixing one is trivially expandable to fixing both. Does that sound right?
| @@ -46,6 +46,11 @@ WHERE | |||
| teamFilter = "AND vat.global_or_team_id = ?" | |||
| } | |||
|
|
|||
| // var tmID uint | |||
| @@ -495,8 +500,8 @@ ON DUPLICATE KEY UPDATE | |||
| if insertOnDuplicateDidInsertOrUpdate(res) { | |||
| id, _ = res.LastInsertId() | |||
| } else { | |||
| stmt := `SELECT id FROM vpp_apps_teams WHERE adam_id = ? AND platform = ?` | |||
| if err := sqlx.GetContext(ctx, tx, &id, stmt, appID.AdamID, appID.Platform); err != nil { | |||
| stmt := `SELECT id FROM vpp_apps_teams WHERE adam_id = ? AND platform = ? AND global_or_team_id = ?` | |||
There was a problem hiding this comment.
Yay for finding another unreleased bug while fixing the first one 😅
| updateAppReq := &updateAppStoreAppRequest{TeamID: &team.ID, SelfService: false, LabelsIncludeAny: []string{lbl2.Name}} | ||
| s.Do("PATCH", fmt.Sprintf("/api/latest/fleet/software/titles/%d/app_store_app", vppAppTitleID), updateAppReq, http.StatusOK) | ||
|
|
||
| // Result should have been cleared for the policy |
There was a problem hiding this comment.
Wait...this doesn't seem right? Moving out of scope shouldn't clear the policy status, since we don't need the reset until the automation might need to fire. Moving back in should. Or am I getting things swapped around?
Checklist for submitter
If some of the following don't apply, delete the relevant line.
SELECT *is avoided, SQL injection is prevented (using placeholders for values in statements)