Skip to content

fix: filter out software from parallels vm #16520

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

Merged
merged 4 commits into from
Feb 5, 2024
Merged

fix: filter out software from parallels vm #16520

merged 4 commits into from
Feb 5, 2024

Conversation

jahzielv
Copy link
Contributor

@jahzielv jahzielv commented Jan 31, 2024

Related issue: #15855

I followed a similar pattern to sanitizeSoftware, a function that modifies the Software. I was originally going to update sanitizeSoftware itself, but decided against it

  1. to avoid making lots of changes to the function signature and internals
  2. because the logic this issue requires is pretty different from what sanitizeSoftware is trying to do, so seemed to warrant its own function.

Checklist for submitter

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

  • Changes file added for user-visible changes in changes/ or orbit/changes/.
    See Changes files for more information.
  • Added/updated tests
  • Manual QA for all new/changed functionality

@jahzielv jahzielv requested a review from a team as a code owner January 31, 2024 22:11
Copy link
Contributor

@roperzh roperzh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very neat implementation! left a comment to discuss!

Copy link

codecov bot commented Jan 31, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (a64cf58) 65.52% compared to head (6546bec) 65.53%.
Report is 29 commits behind head on main.

Files Patch % Lines
server/service/osquery_utils/queries.go 75.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #16520      +/-   ##
==========================================
+ Coverage   65.52%   65.53%   +0.01%     
==========================================
  Files        1132     1132              
  Lines       99069    99077       +8     
  Branches     2448     2448              
==========================================
+ Hits        64916    64933      +17     
+ Misses      29266    29256      -10     
- Partials     4887     4888       +1     
Flag Coverage Δ
backend 66.60% <75.00%> (+0.01%) ⬆️

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.

Copy link
Contributor

@gillespi314 gillespi314 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@jahzielv jahzielv merged commit fa46cfb into main Feb 5, 2024
@jahzielv jahzielv deleted the 15855-vm-software branch February 5, 2024 15:00
georgekarrv pushed a commit that referenced this pull request Feb 9, 2024
> Related issue: #15855

I followed a similar pattern to `sanitizeSoftware`, a function that
modifies the `Software`. I was originally going to update
`sanitizeSoftware` itself, but decided against it
1. to avoid making lots of changes to the function signature and
internals
2. because the logic this issue requires is pretty different from what
`sanitizeSoftware` is trying to do, so seemed to warrant its own
function.

# 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/` or
`orbit/changes/`.
See [Changes
files](https://fleetdm.com/docs/contributing/committing-changes#changes-files)
for more information.
- [x] Added/updated tests
- [x] Manual QA for all new/changed functionality

---------

Co-authored-by: Roberto Dip <[email protected]>
@lukeheath lukeheath added this to the 4.44.1 milestone Feb 13, 2024
georgekarrv pushed a commit that referenced this pull request Feb 13, 2024
> Related issue: #15855

I followed a similar pattern to `sanitizeSoftware`, a function that
modifies the `Software`. I was originally going to update
`sanitizeSoftware` itself, but decided against it
1. to avoid making lots of changes to the function signature and
internals
2. because the logic this issue requires is pretty different from what
`sanitizeSoftware` is trying to do, so seemed to warrant its own
function.

# 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/` or
`orbit/changes/`.
See [Changes
files](https://fleetdm.com/docs/contributing/committing-changes#changes-files)
for more information.
- [x] Added/updated tests
- [x] Manual QA for all new/changed functionality

---------

Co-authored-by: Roberto Dip <[email protected]>
@lukeheath lukeheath removed this from the 4.44.1 milestone Feb 13, 2024
georgekarrv pushed a commit that referenced this pull request Feb 13, 2024
> Related issue: #15855

I followed a similar pattern to `sanitizeSoftware`, a function that
modifies the `Software`. I was originally going to update
`sanitizeSoftware` itself, but decided against it
1. to avoid making lots of changes to the function signature and
internals
2. because the logic this issue requires is pretty different from what
`sanitizeSoftware` is trying to do, so seemed to warrant its own
function.

# 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/` or
`orbit/changes/`.
See [Changes
files](https://fleetdm.com/docs/contributing/committing-changes#changes-files)
for more information.
- [x] Added/updated tests
- [x] Manual QA for all new/changed functionality

---------

Co-authored-by: Roberto Dip <[email protected]>
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.

4 participants