-
Notifications
You must be signed in to change notification settings - Fork 440
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
Optimized software versions endpoint #24496
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #24496 +/- ##
==========================================
- Coverage 63.52% 63.51% -0.01%
==========================================
Files 1591 1592 +1
Lines 151131 151159 +28
Branches 3881 3831 -50
==========================================
+ Hits 96000 96014 +14
- Misses 47483 47493 +10
- Partials 7648 7652 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
7235cd7
to
31e1dd6
Compare
s.DoJSON( | ||
"GET", "/api/latest/fleet/software/versions", | ||
listSoftwareRequest{}, | ||
http.StatusOK, &respVersions, |
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.
When using a premium flag/feature it appears the API returns an error and fleet.ErrMissingLicense
. Wonder if that is something that should be done here as well.
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.
Silently dropping the option for Fleet Free, since theoretically you should be able to run Fleet Free without the EE code, feels fine here.
Also, in order to do the license error you'd need to be able to tell the difference between an unset query param and an explicit false, which would blow up complexity on this a bit, so just as well not to do that.
The software versions endpoint cve details can be truncated using the `without_vulnerability_details` flag.
31e1dd6
to
46eead7
Compare
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.
LGTM other than changes file
s.DoJSON( | ||
"GET", "/api/latest/fleet/software/versions", | ||
listSoftwareRequest{}, | ||
http.StatusOK, &respVersions, |
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.
Silently dropping the option for Fleet Free, since theoretically you should be able to run Fleet Free without the EE code, feels fine here.
Also, in order to do the license error you'd need to be able to tell the difference between an unset query param and an explicit false, which would blow up complexity on this a bit, so just as well not to do that.
Co-authored-by: Ian Littman <[email protected]>
54a965d
to
9ab69d2
Compare
The software versions endpoint cve details can be truncated using the `without_vulnerability_details` flag. #23679 # Checklist for submitter If some of the following don't apply, delete the relevant line. <!-- Note that API documentation changes are now addressed by the product design team. --> - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/Committing-Changes.md#changes-files) for more information. - [x] Added/updated tests - [x] Manual QA for all new/changed functionality --------- Co-authored-by: Ian Littman <[email protected]>
#23679 Original PR: #24496 Co-authored-by: Ian Littman <[email protected]>
The software versions endpoint cve details can be truncated using the
without_vulnerability_details
flag.#23679
Checklist for submitter
If some of the following don't apply, delete the relevant line.
changes/
,orbit/changes/
oree/fleetd-chrome/changes
.See Changes files for more information.