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

Fall back to name/source for matching software titles on insert if bundle ID is provided, add missing special case for edit software upload timeout #22412

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 76 additions & 0 deletions server/datastore/mysql/software_installers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ func TestSoftwareInstallers(t *testing.T) {
{"HasSelfServiceSoftwareInstallers", testHasSelfServiceSoftwareInstallers},
{"DeleteSoftwareInstallersAssignedToPolicy", testDeleteSoftwareInstallersAssignedToPolicy},
{"GetHostLastInstallData", testGetHostLastInstallData},
{"GetOrGenerateSoftwareInstallerTitleID", testGetOrGenerateSoftwareInstallerTitleID},
}

for _, c := range cases {
Expand Down Expand Up @@ -1098,3 +1099,78 @@ func testGetHostLastInstallData(t *testing.T, ds *Datastore) {
require.NoError(t, err)
require.Nil(t, host2LastInstall)
}

func testGetOrGenerateSoftwareInstallerTitleID(t *testing.T, ds *Datastore) {
ctx := context.Background()

host1 := test.NewHost(t, ds, "host1", "", "host1key", "host1uuid", time.Now())
host2 := test.NewHost(t, ds, "host2", "", "host2key", "host2uuid", time.Now())

software1 := []fleet.Software{
{Name: "Existing Title", Version: "0.0.1", Source: "apps", BundleIdentifier: "existing.title"},
}
software2 := []fleet.Software{
{Name: "Existing Title", Version: "v0.0.2", Source: "apps", BundleIdentifier: "existing.title"},
{Name: "Existing Title", Version: "0.0.3", Source: "apps", BundleIdentifier: "existing.title"},
{Name: "Existing Title Without Bundle", Version: "0.0.3", Source: "apps"},
}

_, err := ds.UpdateHostSoftware(ctx, host1.ID, software1)
require.NoError(t, err)
_, err = ds.UpdateHostSoftware(ctx, host2.ID, software2)
require.NoError(t, err)
require.NoError(t, ds.SyncHostsSoftware(ctx, time.Now()))
require.NoError(t, ds.ReconcileSoftwareTitles(ctx))
require.NoError(t, ds.SyncHostsSoftwareTitles(ctx, time.Now()))

tests := []struct {
name string
payload *fleet.UploadSoftwareInstallerPayload
}{
{
name: "title that already exists, no bundle identifier in payload",
payload: &fleet.UploadSoftwareInstallerPayload{
Title: "Existing Title",
Source: "apps",
},
},
{
name: "title that already exists, bundle identifier in payload",
payload: &fleet.UploadSoftwareInstallerPayload{
Title: "Existing Title",
Source: "apps",
BundleIdentifier: "com.existing.bundle",
},
},
{
name: "title that already exists but doesn't have a bundle identifier",
iansltx marked this conversation as resolved.
Show resolved Hide resolved
payload: &fleet.UploadSoftwareInstallerPayload{
Title: "Existing Title Without Bundle",
Source: "apps",
},
},
{
name: "title that doesn't exist, no bundle identifier in payload",
payload: &fleet.UploadSoftwareInstallerPayload{
Title: "New Title",
Source: "some_source",
},
},
{
name: "title that doesn't exist, with bundle identifier in payload",
payload: &fleet.UploadSoftwareInstallerPayload{
Title: "New Title With Bundle",
Source: "some_source",
BundleIdentifier: "com.new.bundle",
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
id, err := ds.getOrGenerateSoftwareInstallerTitleID(ctx, tt.payload)
require.NoError(t, err)
require.NotEmpty(t, id)
})
}
}
25 changes: 17 additions & 8 deletions server/datastore/mysql/vpp.go
Original file line number Diff line number Diff line change
Expand Up @@ -371,17 +371,26 @@ func (ds *Datastore) getOrInsertSoftwareTitleForVPPApp(ctx context.Context, tx s
insertArgs := []any{app.Name, source}

if app.BundleIdentifier != "" {
// NOTE: The index `idx_sw_titles` doesn't include the bundle
// identifier. It's possible for the select to return nothing
// but for the insert to fail if an app with the same name but
// no bundle identifier exists in the DB.
// match by bundle identifier first, or standard matching if we
// don't have a bundle identifier match
switch source {
case "ios_apps", "ipados_apps":
selectStmt = `SELECT id FROM software_titles WHERE bundle_identifier = ? AND source = ?`
selectArgs = []any{app.BundleIdentifier, source}
selectStmt = `
SELECT id
FROM software_titles
WHERE (bundle_identifier = ? AND source = ?) OR (name = ? AND source = ? AND browser = '')
ORDER BY bundle_identifier = ? DESC
LIMIT 1`
selectArgs = []any{app.BundleIdentifier, source, app.Name, source, app.BundleIdentifier}
default:
selectStmt = `SELECT id FROM software_titles WHERE bundle_identifier = ? AND source NOT IN ('ios_apps', 'ipados_apps')`
selectArgs = []any{app.BundleIdentifier}
selectStmt = `
iansltx marked this conversation as resolved.
Show resolved Hide resolved
SELECT id
FROM software_titles
WHERE (bundle_identifier = ? OR (name = ? AND browser = ''))
AND source NOT IN ('ios_apps', 'ipados_apps')
ORDER BY bundle_identifier = ? DESC
LIMIT 1`
selectArgs = []any{app.BundleIdentifier, app.Name, app.BundleIdentifier}
}
insertStmt = `INSERT INTO software_titles (name, source, bundle_identifier, browser) VALUES (?, ?, ?, '')`
insertArgs = append(insertArgs, app.BundleIdentifier)
Expand Down
88 changes: 88 additions & 0 deletions server/datastore/mysql/vpp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package mysql

import (
"context"
"fmt"
"testing"
"time"

Expand Down Expand Up @@ -30,6 +31,7 @@ func TestVPP(t *testing.T) {
{"GetVPPAppByTeamAndTitleID", testGetVPPAppByTeamAndTitleID},
{"VPPTokensCRUD", testVPPTokensCRUD},
{"VPPTokenAppTeamAssociations", testVPPTokenAppTeamAssociations},
{"GetOrInsertSoftwareTitleForVPPApp", testGetOrInsertSoftwareTitleForVPPApp},
}

for _, c := range cases {
Expand Down Expand Up @@ -1201,3 +1203,89 @@ func testVPPTokenAppTeamAssociations(t *testing.T, ds *Datastore) {
_, err = ds.InsertVPPAppWithTeam(ctx, app1, &team2.ID)
assert.Error(t, err)
}

func testGetOrInsertSoftwareTitleForVPPApp(t *testing.T, ds *Datastore) {
ctx := context.Background()

host1 := test.NewHost(t, ds, "host1", "", "host1key", "host1uuid", time.Now())
host2 := test.NewHost(t, ds, "host2", "", "host2key", "host2uuid", time.Now())

software1 := []fleet.Software{
{Name: "Existing Title", Version: "0.0.1", Source: "apps", BundleIdentifier: "existing.title"},
}
software2 := []fleet.Software{
{Name: "Existing Title", Version: "v0.0.2", Source: "apps", BundleIdentifier: "existing.title"},
{Name: "Existing Title", Version: "0.0.3", Source: "apps", BundleIdentifier: "existing.title"},
{Name: "Existing Title Without Bundle", Version: "0.0.3", Source: "apps"},
}

_, err := ds.UpdateHostSoftware(ctx, host1.ID, software1)
require.NoError(t, err)
_, err = ds.UpdateHostSoftware(ctx, host2.ID, software2)
require.NoError(t, err)
require.NoError(t, ds.SyncHostsSoftware(ctx, time.Now()))
require.NoError(t, ds.ReconcileSoftwareTitles(ctx))
require.NoError(t, ds.SyncHostsSoftwareTitles(ctx, time.Now()))

tests := []struct {
name string
app *fleet.VPPApp
}{
{
name: "title that already exists, no bundle identifier in payload",
Copy link
Member Author

Choose a reason for hiding this comment

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

See notes on the installers test cases.

Copy link
Member Author

Choose a reason for hiding this comment

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

Confirmed that this catches bugginess in 4.57.0, and we have a test for mismatched bundle ID in the other test (not this one)...but probably might as well add one more test case since this is a different code path than for installers. Going to work on adding that now.

app: &fleet.VPPApp{
Name: "Existing Title",
LatestVersion: "0.0.1",
BundleIdentifier: "",
},
},
{
name: "title that already exists, bundle identifier in payload",
app: &fleet.VPPApp{
Name: "Existing Title",
LatestVersion: "0.0.2",
BundleIdentifier: "existing.title",
},
},
{
name: "title that already exists but doesn't have a bundle identifier",
app: &fleet.VPPApp{
Name: "Existing Title Without Bundle",
LatestVersion: "0.0.3",
BundleIdentifier: "",
},
},
{
name: "title that doesn't exist, no bundle identifier in payload",
app: &fleet.VPPApp{
Name: "New Title",
LatestVersion: "0.1.0",
BundleIdentifier: "",
},
},
{
name: "title that doesn't exist, with bundle identifier in payload",
app: &fleet.VPPApp{
Name: "New Title",
LatestVersion: "0.1.0",
BundleIdentifier: "new.title.bundle",
},
},
}

for _, platform := range fleet.VPPAppsPlatforms {
for _, tt := range tests {
t.Run(fmt.Sprintf("%s_%v", tt.name, platform), func(t *testing.T) {
tt.app.Platform = platform
var id uint
err := ds.withTx(ctx, func(tx sqlx.ExtContext) error {
var err error
id, err = ds.getOrInsertSoftwareTitleForVPPApp(ctx, tx, tt.app)
return err
})
require.NoError(t, err)
require.NotEmpty(t, id)
})
}
}
}
38 changes: 38 additions & 0 deletions server/service/integration_enterprise_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14366,3 +14366,41 @@ func (s *integrationEnterpriseTestSuite) TestPolicyAutomationsSoftwareInstallers
require.NoError(t, err)
require.Nil(t, hostVanillaOsquery5Team1LastInstall)
}

func (s *integrationEnterpriseTestSuite) TestSoftwareInstallersWithoutBundleIdentifier() {
t := s.T()
ctx := context.Background()

// Create a host without a team
host, err := s.ds.NewHost(context.Background(), &fleet.Host{
DetailUpdatedAt: time.Now(),
LabelUpdatedAt: time.Now(),
PolicyUpdatedAt: time.Now(),
SeenTime: time.Now().Add(-1 * time.Minute),
OsqueryHostID: ptr.String(t.Name()),
NodeKey: ptr.String(t.Name()),
UUID: uuid.New().String(),
Hostname: fmt.Sprintf("%sfoo.local", t.Name()),
Platform: "darwin",
})
require.NoError(t, err)

software := []fleet.Software{
{Name: "DummyApp.app", Version: "0.0.2", Source: "apps"},
}
// we must ingest the title with an empty bundle identifier for this
// test to be valid
require.Empty(t, software[0].BundleIdentifier)
_, err = s.ds.UpdateHostSoftware(ctx, host.ID, software)
require.NoError(t, err)
require.NoError(t, s.ds.SyncHostsSoftware(ctx, time.Now()))
require.NoError(t, s.ds.ReconcileSoftwareTitles(ctx))
require.NoError(t, s.ds.SyncHostsSoftwareTitles(ctx, time.Now()))

payload := &fleet.UploadSoftwareInstallerPayload{
InstallScript: "install",
Filename: "dummy_installer.pkg",
Version: "0.0.2",
}
s.uploadSoftwareInstaller(payload, http.StatusOK, "")
}
Loading