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

Minor fixes to policy software installations (#22148) #22166

Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion cmd/fleetctl/get_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2320,7 +2320,7 @@ func TestGetTeamsYAMLAndApply(t *testing.T) {
declaration.DeclarationUUID = uuid.NewString()
return declaration, nil
}
ds.BatchSetSoftwareInstallersFunc = func(ctx context.Context, tmID *uint, installers []*fleet.UploadSoftwareInstallerPayload) ([]fleet.SoftwareInstaller, error) {
ds.BatchSetSoftwareInstallersFunc = func(ctx context.Context, tmID *uint, installers []*fleet.UploadSoftwareInstallerPayload) ([]fleet.SoftwarePackageResponse, error) {
return nil, nil
}

Expand Down
41 changes: 32 additions & 9 deletions cmd/fleetctl/gitops_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ func TestGitOpsBasicGlobalPremium(t *testing.T) {
ds.NewJobFunc = func(ctx context.Context, job *fleet.Job) (*fleet.Job, error) {
return &fleet.Job{}, nil
}
ds.BatchSetSoftwareInstallersFunc = func(ctx context.Context, teamID *uint, installers []*fleet.UploadSoftwareInstallerPayload) ([]fleet.SoftwareInstaller, error) {
ds.BatchSetSoftwareInstallersFunc = func(ctx context.Context, teamID *uint, installers []*fleet.UploadSoftwareInstallerPayload) ([]fleet.SoftwarePackageResponse, error) {
return nil, nil
}

Expand Down Expand Up @@ -373,7 +373,7 @@ func TestGitOpsBasicTeam(t *testing.T) {
ds.DeleteMDMAppleDeclarationByNameFunc = func(ctx context.Context, teamID *uint, name string) error {
return nil
}
ds.BatchSetSoftwareInstallersFunc = func(ctx context.Context, teamID *uint, installers []*fleet.UploadSoftwareInstallerPayload) ([]fleet.SoftwareInstaller, error) {
ds.BatchSetSoftwareInstallersFunc = func(ctx context.Context, teamID *uint, installers []*fleet.UploadSoftwareInstallerPayload) ([]fleet.SoftwarePackageResponse, error) {
return nil, nil
}
ds.ApplyEnrollSecretsFunc = func(ctx context.Context, teamID *uint, secrets []*fleet.EnrollSecret) error {
Expand Down Expand Up @@ -804,7 +804,7 @@ func TestGitOpsFullTeam(t *testing.T) {
return nil
}
var appliedSoftwareInstallers []*fleet.UploadSoftwareInstallerPayload
ds.BatchSetSoftwareInstallersFunc = func(ctx context.Context, teamID *uint, installers []*fleet.UploadSoftwareInstallerPayload) ([]fleet.SoftwareInstaller, error) {
ds.BatchSetSoftwareInstallersFunc = func(ctx context.Context, teamID *uint, installers []*fleet.UploadSoftwareInstallerPayload) ([]fleet.SoftwarePackageResponse, error) {
appliedSoftwareInstallers = installers
return nil, nil
}
Expand Down Expand Up @@ -1055,7 +1055,7 @@ func TestGitOpsBasicGlobalAndTeam(t *testing.T) {
savedTeam = team
return team, nil
}
ds.BatchSetSoftwareInstallersFunc = func(ctx context.Context, teamID *uint, installers []*fleet.UploadSoftwareInstallerPayload) ([]fleet.SoftwareInstaller, error) {
ds.BatchSetSoftwareInstallersFunc = func(ctx context.Context, teamID *uint, installers []*fleet.UploadSoftwareInstallerPayload) ([]fleet.SoftwarePackageResponse, error) {
return nil, nil
}
ds.ListSoftwareTitlesFunc = func(ctx context.Context, opt fleet.SoftwareTitleListOptions, tmFilter fleet.TeamFilter) ([]fleet.SoftwareTitleListResult, int, *fleet.PaginationMetadata, error) {
Expand Down Expand Up @@ -1317,7 +1317,7 @@ func TestGitOpsBasicGlobalAndNoTeam(t *testing.T) {
savedTeam = team
return team, nil
}
ds.BatchSetSoftwareInstallersFunc = func(ctx context.Context, teamID *uint, installers []*fleet.UploadSoftwareInstallerPayload) ([]fleet.SoftwareInstaller, error) {
ds.BatchSetSoftwareInstallersFunc = func(ctx context.Context, teamID *uint, installers []*fleet.UploadSoftwareInstallerPayload) ([]fleet.SoftwarePackageResponse, error) {
return nil, nil
}
ds.ListSoftwareTitlesFunc = func(ctx context.Context, opt fleet.SoftwareTitleListOptions, tmFilter fleet.TeamFilter) ([]fleet.SoftwareTitleListResult, int, *fleet.PaginationMetadata, error) {
Expand Down Expand Up @@ -1442,6 +1442,20 @@ software:
`)
require.NoError(t, err)

noTeamFilePathPoliciesCalendarPath := filepath.Join(t.TempDir(), "no-team.yml")
noTeamFilePathPoliciesCalendar, err := os.Create(noTeamFilePathPoliciesCalendarPath)
require.NoError(t, err)
_, err = noTeamFilePathPoliciesCalendar.WriteString(`
controls:
policies:
- name: Foobar
query: SELECT 1 FROM osquery_info WHERE start_time < 0;
calendar_events_enabled: true
name: No team
software:
`)
require.NoError(t, err)

noTeamFilePathWithControls := filepath.Join(t.TempDir(), "no-team.yml")
noTeamFileWithControls, err := os.Create(noTeamFilePathWithControls)
require.NoError(t, err)
Expand Down Expand Up @@ -1480,16 +1494,25 @@ software:
require.Error(t, err)
assert.True(t, strings.Contains(err.Error(), "'controls' cannot be set on both global config and on no-team.yml"))
// Real run, both global and no-team.yml define controls.
_, err = runAppNoChecks([]string{"gitops", "-f", globalFileWithControls.Name(), "-f", teamFile.Name(), "-f", noTeamFileWithControls.Name(), "--dry-run"})
_, err = runAppNoChecks([]string{"gitops", "-f", globalFileWithControls.Name(), "-f", teamFile.Name(), "-f", noTeamFileWithControls.Name()})
require.Error(t, err)
assert.True(t, strings.Contains(err.Error(), "'controls' cannot be set on both global config and on no-team.yml"))

// Dry run, both global and no-team.yml defines policy with calendar events enabled.
_, err = runAppNoChecks([]string{"gitops", "-f", globalFileWithControls.Name(), "-f", teamFile.Name(), "-f", noTeamFilePathPoliciesCalendar.Name(), "--dry-run"})
require.Error(t, err)
assert.True(t, strings.Contains(err.Error(), "calendar events are not supported on \"No team\" policies: \"Foobar\""), err.Error())
// Real run, both global and no-team.yml define controls.
_, err = runAppNoChecks([]string{"gitops", "-f", globalFileWithControls.Name(), "-f", teamFile.Name(), "-f", noTeamFilePathPoliciesCalendar.Name()})
require.Error(t, err)
assert.True(t, strings.Contains(err.Error(), "calendar events are not supported on \"No team\" policies: \"Foobar\""), err.Error())

// Dry run, controls should be defined somewhere, either in no-team.yml or global.
_, err = runAppNoChecks([]string{"gitops", "-f", globalFileWithoutControlsAndSoftwareKeys.Name(), "-f", teamFile.Name(), "-f", noTeamFileWithoutControls.Name(), "--dry-run"})
require.Error(t, err)
assert.True(t, strings.Contains(err.Error(), "'controls' must be set on global config or no-team.yml"))
// Real run, both global and no-team.yml define controls.
_, err = runAppNoChecks([]string{"gitops", "-f", globalFileWithoutControlsAndSoftwareKeys.Name(), "-f", teamFile.Name(), "-f", noTeamFileWithoutControls.Name(), "--dry-run"})
_, err = runAppNoChecks([]string{"gitops", "-f", globalFileWithoutControlsAndSoftwareKeys.Name(), "-f", teamFile.Name(), "-f", noTeamFileWithoutControls.Name()})
require.Error(t, err)
assert.True(t, strings.Contains(err.Error(), "'controls' must be set on global config or no-team.yml"))

Expand Down Expand Up @@ -1645,7 +1668,7 @@ func TestGitOpsTeamSoftwareInstallersQueryEnv(t *testing.T) {

t.Setenv("QUERY_VAR", "IT_WORKS")

ds.BatchSetSoftwareInstallersFunc = func(ctx context.Context, tmID *uint, installers []*fleet.UploadSoftwareInstallerPayload) ([]fleet.SoftwareInstaller, error) {
ds.BatchSetSoftwareInstallersFunc = func(ctx context.Context, tmID *uint, installers []*fleet.UploadSoftwareInstallerPayload) ([]fleet.SoftwarePackageResponse, error) {
if installers[0].PreInstallQuery != "select IT_WORKS" {
return nil, fmt.Errorf("Missing env var, got %s", installers[0].PreInstallQuery)
}
Expand Down Expand Up @@ -2158,7 +2181,7 @@ func setupFullGitOpsPremiumServer(t *testing.T) (*mock.Store, **fleet.AppConfig,
declaration.DeclarationUUID = uuid.NewString()
return declaration, nil
}
ds.BatchSetSoftwareInstallersFunc = func(ctx context.Context, teamID *uint, installers []*fleet.UploadSoftwareInstallerPayload) ([]fleet.SoftwareInstaller, error) {
ds.BatchSetSoftwareInstallersFunc = func(ctx context.Context, teamID *uint, installers []*fleet.UploadSoftwareInstallerPayload) ([]fleet.SoftwarePackageResponse, error) {
return nil, nil
}

Expand Down
5 changes: 3 additions & 2 deletions ee/server/service/software_installers.go
Original file line number Diff line number Diff line change
Expand Up @@ -747,7 +747,8 @@ func (svc *Service) UninstallSoftwareTitle(ctx context.Context, hostID uint, sof
}

func (svc *Service) insertSoftwareUninstallRequest(ctx context.Context, executionID string, host *fleet.Host,
installer *fleet.SoftwareInstaller) error {
installer *fleet.SoftwareInstaller,
) error {
if err := svc.ds.InsertSoftwareUninstallRequest(ctx, executionID, host.ID, installer.InstallerID); err != nil {
return ctxerr.Wrap(ctx, err, "inserting software uninstall request")
}
Expand Down Expand Up @@ -892,7 +893,7 @@ const maxInstallerSizeBytes int64 = 1024 * 1024 * 500

func (svc *Service) BatchSetSoftwareInstallers(
ctx context.Context, tmName string, payloads []fleet.SoftwareInstallerPayload, dryRun bool,
) ([]fleet.SoftwareInstaller, error) {
) ([]fleet.SoftwarePackageResponse, error) {
if err := svc.authz.Authorize(ctx, &fleet.Team{}, fleet.ActionRead); err != nil {
return nil, err
}
Expand Down
3 changes: 3 additions & 0 deletions pkg/spec/gitops.go
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,9 @@ func parsePolicies(top map[string]json.RawMessage, result *GitOps, baseDir strin
} else {
item.Team = ""
}
if item.CalendarEventsEnabled && result.IsNoTeam() {
multiError = multierror.Append(multiError, fmt.Errorf("calendar events are not supported on \"No team\" policies: %q", item.Name))
}
}
duplicates := getDuplicateNames(
result.Policies, func(p *GitOpsPolicySpec) string {
Expand Down
2 changes: 1 addition & 1 deletion server/datastore/mysql/activities.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ func (ds *Datastore) ListHostUpcomingActivities(ctx context.Context, hostID uint
'software_package', si.filename,
'install_uuid', hsi.execution_id,
'status', CAST(hsi.status AS CHAR),
'self_service', si.self_service IS TRUE
'self_service', hsi.self_service IS TRUE
) as details
FROM
host_software_installs hsi
Expand Down
16 changes: 4 additions & 12 deletions server/datastore/mysql/software_installers.go
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ func (ds *Datastore) CleanupUnusedSoftwareInstallers(ctx context.Context, softwa
return ctxerr.Wrap(ctx, err, "cleanup unused software installers")
}

func (ds *Datastore) BatchSetSoftwareInstallers(ctx context.Context, tmID *uint, installers []*fleet.UploadSoftwareInstallerPayload) ([]fleet.SoftwareInstaller, error) {
func (ds *Datastore) BatchSetSoftwareInstallers(ctx context.Context, tmID *uint, installers []*fleet.UploadSoftwareInstallerPayload) ([]fleet.SoftwarePackageResponse, error) {
const upsertSoftwareTitles = `
INSERT INTO software_titles
(name, source, browser)
Expand Down Expand Up @@ -752,16 +752,7 @@ ON DUPLICATE KEY UPDATE

const loadInsertedSoftwareInstallers = `
SELECT
id,
team_id,
storage_id,
filename,
version,
install_script_content_id,
pre_install_query,
post_install_script_content_id,
platform,
self_service,
title_id,
url
FROM
Expand All @@ -775,7 +766,7 @@ WHERE global_or_team_id = ?
globalOrTeamID = *tmID
}

var insertedSoftwareInstallers []fleet.SoftwareInstaller
var insertedSoftwareInstallers []fleet.SoftwarePackageResponse
if err := ds.withRetryTxx(ctx, func(tx sqlx.ExtContext) error {
// if no installers are provided, just delete whatever was in
// the table
Expand Down Expand Up @@ -953,7 +944,8 @@ func (ds *Datastore) GetSoftwareInstallersWithoutPackageIDs(ctx context.Context)
}

func (ds *Datastore) UpdateSoftwareInstallerWithoutPackageIDs(ctx context.Context, id uint,
payload fleet.UploadSoftwareInstallerPayload) error {
payload fleet.UploadSoftwareInstallerPayload,
) error {
uninstallScriptID, err := ds.getOrGenerateScriptContentsID(ctx, payload.UninstallScript)
if err != nil {
return ctxerr.Wrap(ctx, err, "get or generate uninstall script contents ID")
Expand Down
21 changes: 14 additions & 7 deletions server/datastore/mysql/software_installers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -651,12 +651,14 @@ func testBatchSetSoftwareInstallers(t *testing.T, ds *Datastore) {
PreInstallQuery: "foo",
UserID: user1.ID,
Platform: "darwin",
URL: "https://example.com",
}})
require.NoError(t, err)
require.Len(t, softwareInstallers, 1)
require.Equal(t, ins0, softwareInstallers[0].Name)
require.NotNil(t, softwareInstallers[0].TeamID)
require.Equal(t, team.ID, *softwareInstallers[0].TeamID)
require.NotNil(t, softwareInstallers[0].TitleID)
require.Equal(t, "darwin", softwareInstallers[0].Platform)
require.Equal(t, "https://example.com", softwareInstallers[0].URL)
assertSoftware([]fleet.SoftwareTitle{
{Name: ins0, Source: "apps", Browser: ""},
})
Expand All @@ -676,6 +678,7 @@ func testBatchSetSoftwareInstallers(t *testing.T, ds *Datastore) {
PreInstallQuery: "select 0 from foo;",
UserID: user1.ID,
Platform: "darwin",
URL: "https://example.com",
},
{
InstallScript: "install",
Expand All @@ -689,16 +692,19 @@ func testBatchSetSoftwareInstallers(t *testing.T, ds *Datastore) {
PreInstallQuery: "select 1 from bar;",
UserID: user1.ID,
Platform: "darwin",
URL: "https://example2.com",
},
})
require.NoError(t, err)
require.Len(t, softwareInstallers, 2)
require.Equal(t, ins0, softwareInstallers[0].Name)
require.NotNil(t, softwareInstallers[0].TitleID)
require.Equal(t, "darwin", softwareInstallers[0].Platform)
require.Equal(t, ins1, softwareInstallers[1].Name)
require.NotNil(t, softwareInstallers[0].TeamID)
require.Equal(t, team.ID, *softwareInstallers[0].TeamID)
require.Equal(t, "https://example.com", softwareInstallers[0].URL)
require.NotNil(t, softwareInstallers[1].TitleID)
require.Equal(t, "darwin", softwareInstallers[1].Platform)
require.NotNil(t, softwareInstallers[1].TeamID)
require.Equal(t, team.ID, *softwareInstallers[1].TeamID)
require.Equal(t, "https://example2.com", softwareInstallers[1].URL)
assertSoftware([]fleet.SoftwareTitle{
{Name: ins0, Source: "apps", Browser: ""},
{Name: ins1, Source: "apps", Browser: ""},
Expand All @@ -721,8 +727,9 @@ func testBatchSetSoftwareInstallers(t *testing.T, ds *Datastore) {
})
require.NoError(t, err)
require.Len(t, softwareInstallers, 1)
require.Equal(t, ins1, softwareInstallers[0].Name)
require.NotNil(t, softwareInstallers[0].TitleID)
require.NotNil(t, softwareInstallers[0].TeamID)
require.Empty(t, softwareInstallers[0].URL)
assertSoftware([]fleet.SoftwareTitle{
{Name: ins1, Source: "apps", Browser: ""},
})
Expand Down
2 changes: 1 addition & 1 deletion server/fleet/datastore.go
Original file line number Diff line number Diff line change
Expand Up @@ -1695,7 +1695,7 @@ type Datastore interface {
CleanupUnusedSoftwareInstallers(ctx context.Context, softwareInstallStore SoftwareInstallerStore, removeCreatedBefore time.Time) error

// BatchSetSoftwareInstallers sets the software installers for the given team or no team.
BatchSetSoftwareInstallers(ctx context.Context, tmID *uint, installers []*UploadSoftwareInstallerPayload) ([]SoftwareInstaller, error)
BatchSetSoftwareInstallers(ctx context.Context, tmID *uint, installers []*UploadSoftwareInstallerPayload) ([]SoftwarePackageResponse, error)

// HasSelfServiceSoftwareInstallers returns true if self-service software installers are available for the team or globally.
HasSelfServiceSoftwareInstallers(ctx context.Context, platform string, teamID *uint) (bool, error)
Expand Down
2 changes: 1 addition & 1 deletion server/fleet/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,7 @@ type Service interface {

// BatchSetSoftwareInstallers replaces the software installers for a specified team.
// Returns the metadata of inserted software installers.
BatchSetSoftwareInstallers(ctx context.Context, tmName string, payloads []SoftwareInstallerPayload, dryRun bool) ([]SoftwareInstaller, error)
BatchSetSoftwareInstallers(ctx context.Context, tmName string, payloads []SoftwareInstallerPayload, dryRun bool) ([]SoftwarePackageResponse, error)

// SelfServiceInstallSoftwareTitle installs a software title
// initiated by the user
Expand Down
11 changes: 11 additions & 0 deletions server/fleet/software_installer.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,17 @@ type SoftwareInstaller struct {
URL string `json:"url" db:"url"`
}

// SoftwarePackageResponse is the response type used when applying software by batch.
type SoftwarePackageResponse struct {
// TeamID is the ID of the team.
// A value of nil means it is scoped to hosts that are assigned to "No team".
TeamID *uint `json:"team_id" db:"team_id"`
// TitleID is the id of the software title associated with the software installer.
TitleID *uint `json:"title_id" db:"title_id"`
// URL is the source URL for this installer (set when uploading via batch/gitops).
URL string `json:"url" db:"url"`
}

// AuthzType implements authz.AuthzTyper.
func (s *SoftwareInstaller) AuthzType() string {
return "installable_entity"
Expand Down
4 changes: 2 additions & 2 deletions server/mock/datastore_mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -1064,7 +1064,7 @@ type GetSoftwareInstallResultsFunc func(ctx context.Context, resultsUUID string)

type CleanupUnusedSoftwareInstallersFunc func(ctx context.Context, softwareInstallStore fleet.SoftwareInstallerStore, removeCreatedBefore time.Time) error

type BatchSetSoftwareInstallersFunc func(ctx context.Context, tmID *uint, installers []*fleet.UploadSoftwareInstallerPayload) ([]fleet.SoftwareInstaller, error)
type BatchSetSoftwareInstallersFunc func(ctx context.Context, tmID *uint, installers []*fleet.UploadSoftwareInstallerPayload) ([]fleet.SoftwarePackageResponse, error)

type HasSelfServiceSoftwareInstallersFunc func(ctx context.Context, platform string, teamID *uint) (bool, error)

Expand Down Expand Up @@ -6333,7 +6333,7 @@ func (s *DataStore) CleanupUnusedSoftwareInstallers(ctx context.Context, softwar
return s.CleanupUnusedSoftwareInstallersFunc(ctx, softwareInstallStore, removeCreatedBefore)
}

func (s *DataStore) BatchSetSoftwareInstallers(ctx context.Context, tmID *uint, installers []*fleet.UploadSoftwareInstallerPayload) ([]fleet.SoftwareInstaller, error) {
func (s *DataStore) BatchSetSoftwareInstallers(ctx context.Context, tmID *uint, installers []*fleet.UploadSoftwareInstallerPayload) ([]fleet.SoftwarePackageResponse, error) {
s.mu.Lock()
s.BatchSetSoftwareInstallersFuncInvoked = true
s.mu.Unlock()
Expand Down
Loading
Loading