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

Allow automatic creation of software install policy for VPP and FMA apps in API #26440

Merged
merged 8 commits into from
Feb 22, 2025

Conversation

iansltx
Copy link
Member

@iansltx iansltx commented Feb 18, 2025

For #26190. FMA is included here because the previous implementation was client-side. Does not include timestamp fields for VPP apps.

Checklist for submitter

If some of the following don't apply, delete the relevant line.

  • Changes file added for user-visible changes in changes/, orbit/changes/ or ee/fleetd-chrome/changes.
    See Changes files for more information.
  • Input data is properly validated, SELECT * is avoided, SQL injection is prevented (using placeholders for values in statements)
  • Added/updated automated tests
  • A detailed QA plan exists on the associated ticket (if it isn't there, work with the product group's QA engineer to add it)
  • Manual QA for all new/changed functionality

For #26190. Also refactors automatic policy generation so we're reusing a bit of the custom package logic, and can reuse further when doing the same thing for FMA.

TODO:

* Tests on new code surface
* Potentially pull auto-install into VPPAppsTeams payload to minimize test diff
* Add same ability to FMA
* Add created_at/updated_at to VPP apps teams + migration backfill
Note that this does *not* use the queries in apps.json, as those queries are identical to what we would build programmatically by bundle ID, and this is a smaller change. We'll almost certainly need to revise this for Windows and/or FMAv2, so going with a reliable but closed-ended change for now.
Copy link

codecov bot commented Feb 18, 2025

Codecov Report

Attention: Patch coverage is 58.99281% with 57 lines in your changes missing coverage. Please review.

Project coverage is 63.85%. Comparing base (f6f540b) to head (1d1f625).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
pkg/automatic_policy/automatic_policy.go 62.96% 26 Missing and 4 partials ⚠️
server/datastore/mysql/vpp.go 0.00% 10 Missing and 1 partial ⚠️
ee/server/service/vpp.go 20.00% 6 Missing and 2 partials ⚠️
server/datastore/mysql/software_installers.go 75.86% 3 Missing and 4 partials ⚠️
server/service/maintained_apps.go 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #26440      +/-   ##
==========================================
- Coverage   63.87%   63.85%   -0.02%     
==========================================
  Files        1662     1662              
  Lines      159264   159331      +67     
  Branches     4125     4125              
==========================================
+ Hits       101724   101736      +12     
- Misses      49600    49645      +45     
- Partials     7940     7950      +10     
Flag Coverage Δ
backend 64.65% <58.99%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@iansltx
Copy link
Member Author

iansltx commented Feb 21, 2025

Tested this end-to-end. So I'm not holding up the FE, I'm planning on getting automated tests on this done post-merge, over the weekend. Will set this to ready once I fix conflicts and add a changes file.

@iansltx iansltx marked this pull request as ready for review February 21, 2025 21:31
@iansltx iansltx requested a review from a team as a code owner February 21, 2025 21:31
@iansltx iansltx merged commit ce36352 into main Feb 22, 2025
37 checks passed
@iansltx iansltx deleted the 26190-vpp-autoinstall-be branch February 22, 2025 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants