-
Notifications
You must be signed in to change notification settings - Fork 452
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
feat: filter host software by label scoping #24801
Changes from all commits
c62f339
ffa3db9
02a87b3
ce9f1a9
88aa527
75eb1a0
8683f48
a4317fc
1f5d82d
ef5ecbf
d3e23ee
a43c6c3
02a4c57
fee783e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
- Add functionality to filter host software based on label scoping. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -69,6 +69,7 @@ func TestSoftware(t *testing.T) { | |
{"ListHostSoftwareInstallThenTransferTeam", testListHostSoftwareInstallThenTransferTeam}, | ||
{"ListHostSoftwareInstallThenDeleteInstallers", testListHostSoftwareInstallThenDeleteInstallers}, | ||
{"ListSoftwareVersionsVulnerabilityFilters", testListSoftwareVersionsVulnerabilityFilters}, | ||
{"TestListHostSoftwareWithLabelScoping", testListHostSoftwareWithLabelScoping}, | ||
} | ||
for _, c := range cases { | ||
t.Run(c.name, func(t *testing.T) { | ||
|
@@ -5246,3 +5247,158 @@ func testListSoftwareVersionsVulnerabilityFilters(t *testing.T, ds *Datastore) { | |
}) | ||
} | ||
} | ||
|
||
func testListHostSoftwareWithLabelScoping(t *testing.T, ds *Datastore) { | ||
ctx := context.Background() | ||
|
||
// create a host | ||
host := test.NewHost(t, ds, "host1", "", "host1key", "host1uuid", time.Now(), test.WithPlatform("darwin")) | ||
nanoEnroll(t, ds, host, false) | ||
user1 := test.NewUser(t, ds, "Alice", "alice@example.com", true) | ||
|
||
// create some software: custom installers and FMA | ||
tfr1, err := fleet.NewTempFileReader(strings.NewReader("hello"), t.TempDir) | ||
require.NoError(t, err) | ||
installerID1, _, err := ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ | ||
InstallScript: "hello", | ||
PreInstallQuery: "SELECT 1", | ||
PostInstallScript: "world", | ||
UninstallScript: "goodbye", | ||
InstallerFile: tfr1, | ||
StorageID: "storage1", | ||
Filename: "file1", | ||
Title: "file1", | ||
Version: "1.0", | ||
Source: "apps", | ||
UserID: user1.ID, | ||
BundleIdentifier: "bi1", | ||
Platform: "darwin", | ||
}) | ||
require.NoError(t, err) | ||
|
||
// we should see installer1, since it has no label associated yet | ||
opts := fleet.HostSoftwareTitleListOptions{ | ||
ListOptions: fleet.ListOptions{ | ||
PerPage: 11, | ||
IncludeMetadata: true, | ||
OrderKey: "name", | ||
TestSecondaryOrderKey: "source", | ||
}, | ||
IncludeAvailableForInstall: true, | ||
} | ||
software, _, err := ds.ListHostSoftware(ctx, host, opts) | ||
require.NoError(t, err) | ||
require.Len(t, software, 1) | ||
require.Equal(t, "file1", software[0].SoftwarePackage.Name) | ||
|
||
label1, err := ds.NewLabel(ctx, &fleet.Label{Name: "label1" + t.Name()}) | ||
require.NoError(t, err) | ||
|
||
// assign the label to the host | ||
require.NoError(t, ds.AddLabelsToHost(ctx, host.ID, []uint{label1.ID})) | ||
|
||
// assign the label to the software installer | ||
// TODO(JVE): update this once the DS method exists | ||
updateInstallerLabel := func(siID, labelID uint, exclude bool) { | ||
ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { | ||
_, err = q.ExecContext( | ||
ctx, | ||
`INSERT INTO software_installer_labels (software_installer_id, label_id, exclude) VALUES (?, ?, ?) ON DUPLICATE KEY UPDATE exclude = VALUES(exclude)`, | ||
siID, labelID, exclude, | ||
) | ||
return err | ||
}) | ||
} | ||
updateInstallerLabel(installerID1, label1.ID, true) | ||
|
||
// should be empty as the installer label is "exclude any" | ||
software, _, err = ds.ListHostSoftware(ctx, host, opts) | ||
require.NoError(t, err) | ||
require.Empty(t, software) | ||
|
||
// Update the label to be "include any" | ||
updateInstallerLabel(installerID1, label1.ID, false) | ||
|
||
software, _, err = ds.ListHostSoftware(ctx, host, opts) | ||
require.NoError(t, err) | ||
require.Len(t, software, 1) | ||
require.Equal(t, "file1", software[0].SoftwarePackage.Name) | ||
|
||
// Add an installer. No label yet. | ||
installerID2, _, err := ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ | ||
InstallScript: "hello", | ||
PreInstallQuery: "SELECT 1", | ||
PostInstallScript: "world", | ||
UninstallScript: "goodbye", | ||
InstallerFile: tfr1, | ||
StorageID: "storage2", | ||
Filename: "file2", | ||
Title: "file2", | ||
Version: "2.0", | ||
Source: "apps", | ||
UserID: user1.ID, | ||
BundleIdentifier: "bi2", | ||
Platform: "darwin", | ||
}) | ||
require.NoError(t, err) | ||
|
||
// There's 2 installers now: installerID1 and installerID2 (because it has no labels associated) | ||
software, _, err = ds.ListHostSoftware(ctx, host, opts) | ||
require.NoError(t, err) | ||
require.Len(t, software, 2) | ||
|
||
// Add "exclude any" labels to installer2 | ||
label2, err := ds.NewLabel(ctx, &fleet.Label{Name: "label2" + t.Name()}) | ||
require.NoError(t, err) | ||
|
||
label3, err := ds.NewLabel(ctx, &fleet.Label{Name: "label3" + t.Name()}) | ||
require.NoError(t, err) | ||
|
||
updateInstallerLabel(installerID2, label2.ID, true) | ||
updateInstallerLabel(installerID2, label3.ID, true) | ||
|
||
// Now host has label1, label2 | ||
require.NoError(t, ds.AddLabelsToHost(ctx, host.ID, []uint{label2.ID})) | ||
|
||
// List should be back to 1 | ||
software, _, err = ds.ListHostSoftware(ctx, host, opts) | ||
require.NoError(t, err) | ||
require.Len(t, software, 1) | ||
|
||
// Add an installer. No label yet. | ||
installerID3, _, err := ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ | ||
InstallScript: "hello", | ||
PreInstallQuery: "SELECT 1", | ||
PostInstallScript: "world", | ||
UninstallScript: "goodbye", | ||
InstallerFile: tfr1, | ||
StorageID: "storage3", | ||
Filename: "file3", | ||
Title: "file3", | ||
Version: "3.0", | ||
Source: "apps", | ||
UserID: user1.ID, | ||
BundleIdentifier: "bi3", | ||
Platform: "darwin", | ||
}) | ||
require.NoError(t, err) | ||
|
||
// Add a new label and apply it to the installer. There are no hosts with this label. | ||
label4, err := ds.NewLabel(ctx, &fleet.Label{Name: "label4" + t.Name()}) | ||
require.NoError(t, err) | ||
|
||
updateInstallerLabel(installerID3, label4.ID, true) | ||
|
||
// We should have [installerID1, installerID3] | ||
software, _, err = ds.ListHostSoftware(ctx, host, opts) | ||
require.NoError(t, err) | ||
require.Len(t, software, 2) | ||
|
||
// Now include hosts with label4. No host has this label, so we shouldn't see installerID3 anymore. | ||
updateInstallerLabel(installerID3, label4.ID, false) | ||
|
||
// We should have [installerID1] | ||
software, _, err = ds.ListHostSoftware(ctx, host, opts) | ||
require.NoError(t, err) | ||
require.Len(t, software, 1) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. here and elsewhere I could probably tighten up the checks by validating the IDs returned. Since there is other testing fixup to do, I will tackle this in a separate PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably best to add it to the catch-all ticket too so we don't forget about this. |
||
} |
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.
One thing I did not think of mentioning earlier, the
exclude_any
case will suffer from the same issue as for the configuration profiles - for dynamic labels (based on a query result), all hosts appear to not be members until we ingest the first result for that host. So this could show an installer as "available" for a host even though once we ingest the dynamic label query results, it should've been excluded. This was fixed with timestamp checks for config profiles.I think it'd be fine to merge as-is and worry about that exclude-any corner case later on, maybe as a "holistic" approach for the whole story? Would you mind adding it to the catch-all ticket if that approach make sense to you?
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.
aha, that's a good point. I can add it to the catch all.