Skip to content

add host additional info filters#28

Merged
zwass merged 3 commits into
fleetdm:masterfrom
billcobbler:add-host-additional-info-filters
Nov 14, 2020
Merged

add host additional info filters#28
zwass merged 3 commits into
fleetdm:masterfrom
billcobbler:add-host-additional-info-filters

Conversation

@billcobbler

@billcobbler billcobbler commented Nov 6, 2020

Copy link
Copy Markdown
Contributor

This change adds the ability to filter additional host info via the list hosts endpoint; a continuation from here, but now filtering is accomplished via SQL.

Additional object without filter:

curl 'https://localhost:8080/api/v1/kolide/hosts'
...
"additional": {
        "macs": [
          {
            "mac": "00:00:00:00:00:00"
          },
          {
            "mac": "02:42:c0:a8:10:05"
          }
        ],
        "time": [
          {
            "day": "13",
            "hour": "3",
            "year": "2020",
            "month": "10",
            "minutes": "43",
            "seconds": "11",
            "weekday": "Tuesday",
            "datetime": "2020-10-13T03:43:11Z",
            "iso_8601": "2020-10-13T03:43:11Z",
            "timezone": "GMT",
            "timestamp": "Tue Oct 13 03:43:11 2020 UTC",
            "unix_time": "1602560591",
            "local_time": "1602560591",
            "local_timezone": "UTC"
          }
},
...

Additional object with filter:

curl 'https://localhost:8080/api/v1/kolide/hosts?additional_info_filters=macs,notreal'
...
"additional": {
        "macs": [
          {
            "mac": "00:00:00:00:00:00"
          },
          {
            "mac": "02:42:c0:a8:10:05"
          }
        ],
        "notreal": null
},
...

Comment thread server/datastore/mysql/hosts.go Outdated
@billcobbler billcobbler requested a review from zwass November 13, 2020 18:32

@zwass zwass left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for adding the SQLi fix!

I think the implementation is ready to merge. Before we do that can you please add a test in https://github.com/fleetdm/fleet/blob/master/server/datastore/datastore_hosts_test.go?

low, high := d.getLimitOffsetSliceBounds(opt.ListOptions, len(hosts))
hosts = hosts[low:high]

// Filter additional info

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In the future please feel free not to implement anything in inmem. It's been mostly removed from use and I need to finish tearing it out.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added tests, but did you also want me to remove this inmem implementation?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No need for that. I'll be removing all of this soon in any case.

@zwass zwass left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks great! Thank you for adding the test. I also tested manually in my local Fleet install and it worked as documented.

low, high := d.getLimitOffsetSliceBounds(opt.ListOptions, len(hosts))
hosts = hosts[low:high]

// Filter additional info

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No need for that. I'll be removing all of this soon in any case.

@zwass zwass merged commit 618ba56 into fleetdm:master Nov 14, 2020
@zwass

zwass commented Nov 25, 2020

Copy link
Copy Markdown
Member

This resolves kolide/fleet#2329.

chiiph pushed a commit that referenced this pull request Aug 4, 2021
Use permissive file permissions to allow mounting files into Wix Docker container.

Fixes #1424
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