Skip to content

Commit

Permalink
25880 vpp labels gitops (#25953)
Browse files Browse the repository at this point in the history
> For #25880 

# 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] Input data is properly validated, `SELECT *` is avoided, SQL
injection is prevented (using placeholders for values in statements)
- [x] Added/updated automated tests
- [x] Manual QA for all new/changed functionality
  • Loading branch information
jahzielv authored Feb 1, 2025
1 parent b498a2a commit 053549f
Show file tree
Hide file tree
Showing 14 changed files with 372 additions and 25 deletions.
38 changes: 30 additions & 8 deletions cmd/fleetctl/gitops_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2200,15 +2200,21 @@ func TestGitOpsTeamVPPApps(t *testing.T) {
file string
wantErr string
tokenExpiration time.Time
expectedLabels map[string]uint
}{
{"testdata/gitops/team_vpp_valid_app.yml", "", time.Now().Add(24 * time.Hour)},
{"testdata/gitops/team_vpp_valid_app_self_service.yml", "", time.Now().Add(24 * time.Hour)},
{"testdata/gitops/team_vpp_valid_empty.yml", "", time.Now().Add(24 * time.Hour)},
{"testdata/gitops/team_vpp_valid_empty.yml", "", time.Now().Add(-24 * time.Hour)},
{"testdata/gitops/team_vpp_valid_app.yml", "VPP token expired", time.Now().Add(-24 * time.Hour)},
{"testdata/gitops/team_vpp_invalid_app.yml", "app not available on vpp account", time.Now().Add(24 * time.Hour)},
{"testdata/gitops/team_vpp_incorrect_type.yml", "\"app_store_apps.app_store_id\" must be a string, found number", time.Now().Add(24 * time.Hour)},
{"testdata/gitops/team_vpp_empty_adamid.yml", "software app store id required", time.Now().Add(24 * time.Hour)},
{"testdata/gitops/team_vpp_valid_app.yml", "", time.Now().Add(24 * time.Hour), map[string]uint{}},
{"testdata/gitops/team_vpp_valid_app_self_service.yml", "", time.Now().Add(24 * time.Hour), map[string]uint{}},
{"testdata/gitops/team_vpp_valid_empty.yml", "", time.Now().Add(24 * time.Hour), map[string]uint{}},
{"testdata/gitops/team_vpp_valid_empty.yml", "", time.Now().Add(-24 * time.Hour), map[string]uint{}},
{"testdata/gitops/team_vpp_valid_app.yml", "VPP token expired", time.Now().Add(-24 * time.Hour), map[string]uint{}},
{"testdata/gitops/team_vpp_invalid_app.yml", "app not available on vpp account", time.Now().Add(24 * time.Hour), map[string]uint{}},
{"testdata/gitops/team_vpp_incorrect_type.yml", "\"app_store_apps.app_store_id\" must be a string, found number", time.Now().Add(24 * time.Hour), map[string]uint{}},
{"testdata/gitops/team_vpp_empty_adamid.yml", "software app store id required", time.Now().Add(24 * time.Hour), map[string]uint{}},
{"testdata/gitops/team_vpp_valid_app_labels_exclude_any.yml", "", time.Now().Add(24 * time.Hour), map[string]uint{"label 1": 1, "label 2": 2}},
{"testdata/gitops/team_vpp_valid_app_labels_include_any.yml", "", time.Now().Add(24 * time.Hour), map[string]uint{"label 1": 1, "label 2": 2}},
{"testdata/gitops/team_vpp_invalid_app_labels_exclude_any.yml", "some or all the labels provided don't exist", time.Now().Add(24 * time.Hour), map[string]uint{"label 1": 1, "label 2": 2}},
{"testdata/gitops/team_vpp_invalid_app_labels_include_any.yml", "some or all the labels provided don't exist", time.Now().Add(24 * time.Hour), map[string]uint{"label 1": 1, "label 2": 2}},
{"testdata/gitops/team_vpp_invalid_app_labels_both.yml", `only one of "labels_exclude_any" or "labels_include_any" can be specified for app store app`, time.Now().Add(24 * time.Hour), map[string]uint{}},
}

for _, c := range cases {
Expand Down Expand Up @@ -2238,9 +2244,25 @@ func TestGitOpsTeamVPPApps(t *testing.T) {
}, nil
}

found := make(map[string]uint)
ds.LabelIDsByNameFunc = func(ctx context.Context, labels []string) (map[string]uint, error) {
for _, l := range labels {
if id, ok := c.expectedLabels[l]; ok {
found[l] = id
}
}
return found, nil
}

_, err = runAppNoChecks([]string{"gitops", "-f", c.file})

if c.wantErr == "" {
require.NoError(t, err)
if len(c.expectedLabels) > 0 {
require.True(t, ds.LabelIDsByNameFuncInvoked)
}

require.Equal(t, c.expectedLabels, found)
} else {
require.ErrorContains(t, err, c.wantErr)
}
Expand Down
21 changes: 21 additions & 0 deletions cmd/fleetctl/testdata/gitops/team_vpp_invalid_app_labels_both.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
name: "${TEST_TEAM_NAME}"
team_settings:
secrets:
- secret: "ABC"
features:
enable_host_users: true
enable_software_inventory: true
host_expiry_settings:
host_expiry_enabled: true
host_expiry_window: 30
agent_options:
controls:
policies:
queries:
software:
app_store_apps:
- app_store_id: "1"
labels_exclude_any:
- "label 1"
labels_include_any:
- "label 2"
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
name: "${TEST_TEAM_NAME}"
team_settings:
secrets:
- secret: "ABC"
features:
enable_host_users: true
enable_software_inventory: true
host_expiry_settings:
host_expiry_enabled: true
host_expiry_window: 30
agent_options:
controls:
policies:
queries:
software:
app_store_apps:
- app_store_id: "1"
labels_exclude_any:
- "label non-existent"
- "label 2"
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
name: "${TEST_TEAM_NAME}"
team_settings:
secrets:
- secret: "ABC"
features:
enable_host_users: true
enable_software_inventory: true
host_expiry_settings:
host_expiry_enabled: true
host_expiry_window: 30
agent_options:
controls:
policies:
queries:
software:
app_store_apps:
- app_store_id: "1"
labels_include_any:
- "label non-existent"
- "label 2"
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
name: "${TEST_TEAM_NAME}"
team_settings:
secrets:
- secret: "ABC"
features:
enable_host_users: true
enable_software_inventory: true
host_expiry_settings:
host_expiry_enabled: true
host_expiry_window: 30
agent_options:
controls:
policies:
queries:
software:
app_store_apps:
- app_store_id: "1"
labels_exclude_any:
- "label 1"
- "label 2"
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
name: "${TEST_TEAM_NAME}"
team_settings:
secrets:
- secret: "ABC"
features:
enable_host_users: true
enable_software_inventory: true
host_expiry_settings:
host_expiry_enabled: true
host_expiry_window: 30
agent_options:
controls:
policies:
queries:
software:
app_store_apps:
- app_store_id: "1"
labels_include_any:
- "label 1"
- "label 2"
28 changes: 20 additions & 8 deletions ee/server/service/vpp.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,18 +70,24 @@ func (svc *Service) BatchAssociateVPPApps(ctx context.Context, teamName string,
// Currently only macOS is supported for self-service. Don't
// import vpp apps as self-service for ios or ipados
payloadsWithPlatform = append(payloadsWithPlatform, []fleet.VPPBatchPayloadWithPlatform{{
AppStoreID: payload.AppStoreID,
SelfService: false,
Platform: fleet.IOSPlatform,
AppStoreID: payload.AppStoreID,
SelfService: false,
Platform: fleet.IOSPlatform,
LabelsExcludeAny: payload.LabelsExcludeAny,
LabelsIncludeAny: payload.LabelsIncludeAny,
}, {
AppStoreID: payload.AppStoreID,
SelfService: false,
Platform: fleet.IPadOSPlatform,
AppStoreID: payload.AppStoreID,
SelfService: false,
Platform: fleet.IPadOSPlatform,
LabelsExcludeAny: payload.LabelsExcludeAny,
LabelsIncludeAny: payload.LabelsIncludeAny,
}, {
AppStoreID: payload.AppStoreID,
SelfService: payload.SelfService,
Platform: fleet.MacOSPlatform,
InstallDuringSetup: payload.InstallDuringSetup,
LabelsExcludeAny: payload.LabelsExcludeAny,
LabelsIncludeAny: payload.LabelsIncludeAny,
}}...)
}

Expand All @@ -107,13 +113,18 @@ func (svc *Service) BatchAssociateVPPApps(ctx context.Context, teamName string,
return nil, fleet.NewInvalidArgumentError("app_store_apps.platform",
fmt.Sprintf("platform must be one of '%s', '%s', or '%s", fleet.IOSPlatform, fleet.IPadOSPlatform, fleet.MacOSPlatform))
}
validatedLabels, err := ValidateSoftwareLabels(ctx, svc, payload.LabelsIncludeAny, payload.LabelsExcludeAny)
if err != nil {
return nil, ctxerr.Wrap(ctx, err, "validating software labels for batch adding vpp app")
}
vppAppTeams = append(vppAppTeams, fleet.VPPAppTeam{
VPPAppID: fleet.VPPAppID{
AdamID: payload.AppStoreID,
Platform: payload.Platform,
},
SelfService: payload.SelfService,
InstallDuringSetup: payload.InstallDuringSetup,
ValidatedLabels: validatedLabels,
})
}

Expand Down Expand Up @@ -408,9 +419,9 @@ func getVPPAppsMetadata(ctx context.Context, ids []fleet.VPPAppTeam) ([]*fleet.V
for _, id := range ids {
if _, ok := adamIDMap[id.AdamID]; !ok {
adamIDMap[id.AdamID] = make(map[fleet.AppleDevicePlatform]fleet.VPPAppTeam, 1)
adamIDMap[id.AdamID][id.Platform] = fleet.VPPAppTeam{SelfService: id.SelfService, InstallDuringSetup: id.InstallDuringSetup}
adamIDMap[id.AdamID][id.Platform] = fleet.VPPAppTeam{SelfService: id.SelfService, InstallDuringSetup: id.InstallDuringSetup, ValidatedLabels: id.ValidatedLabels}
} else {
adamIDMap[id.AdamID][id.Platform] = fleet.VPPAppTeam{SelfService: id.SelfService, InstallDuringSetup: id.InstallDuringSetup}
adamIDMap[id.AdamID][id.Platform] = fleet.VPPAppTeam{SelfService: id.SelfService, InstallDuringSetup: id.InstallDuringSetup, ValidatedLabels: id.ValidatedLabels}
}
}

Expand All @@ -435,6 +446,7 @@ func getVPPAppsMetadata(ctx context.Context, ids []fleet.VPPAppTeam) ([]*fleet.V
},
SelfService: props.SelfService,
InstallDuringSetup: props.InstallDuringSetup,
ValidatedLabels: props.ValidatedLabels,
},
BundleIdentifier: metadata.BundleID,
IconURL: metadata.ArtworkURL,
Expand Down
6 changes: 6 additions & 0 deletions pkg/spec/gitops.go
Original file line number Diff line number Diff line change
Expand Up @@ -848,6 +848,12 @@ func parseSoftware(top map[string]json.RawMessage, result *GitOps, baseDir strin
multiError = multierror.Append(multiError, errors.New("software app store id required"))
continue
}

if len(item.LabelsExcludeAny) > 0 && len(item.LabelsIncludeAny) > 0 {
multiError = multierror.Append(multiError, fmt.Errorf(`only one of "labels_exclude_any" or "labels_include_any" can be specified for app store app %q`, item.AppStoreID))
continue
}

result.Software.AppStoreApps = append(result.Software.AppStoreApps, &item)
}
for _, item := range software.Packages {
Expand Down
71 changes: 67 additions & 4 deletions server/datastore/mysql/vpp.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,48 @@ func (ds *Datastore) BatchInsertVPPApps(ctx context.Context, apps []*fleet.VPPAp
})
}

func (ds *Datastore) getExistingLabels(ctx context.Context, vppAppTeamID uint) (*fleet.LabelIdentsWithScope, error) {
existingLabels, err := ds.getVPPAppLabels(ctx, vppAppTeamID)
if err != nil {
return nil, ctxerr.Wrap(ctx, err, "getting existing labels")
}

var labels fleet.LabelIdentsWithScope
var exclAny, inclAny []fleet.SoftwareScopeLabel
for _, l := range existingLabels {
if l.Exclude {
exclAny = append(exclAny, l)
} else {
inclAny = append(inclAny, l)
}
}

if len(inclAny) > 0 && len(exclAny) > 0 {
// there's a bug somewhere
return nil, ctxerr.New(ctx, "found both include and exclude labels on a vpp app")
}

switch {
case len(exclAny) > 0:
labels.LabelScope = fleet.LabelScopeExcludeAny
labels.ByName = make(map[string]fleet.LabelIdent, len(exclAny))
for _, l := range exclAny {
labels.ByName[l.LabelName] = fleet.LabelIdent{LabelName: l.LabelName, LabelID: l.LabelID}
}
return &labels, nil

case len(inclAny) > 0:
labels.LabelScope = fleet.LabelScopeExcludeAny
labels.ByName = make(map[string]fleet.LabelIdent, len(inclAny))
for _, l := range inclAny {
labels.ByName[l.LabelName] = fleet.LabelIdent{LabelName: l.LabelName, LabelID: l.LabelID}
}
return &labels, nil
default:
return nil, nil
}
}

func (ds *Datastore) SetTeamVPPApps(ctx context.Context, teamID *uint, appFleets []fleet.VPPAppTeam) error {
existingApps, err := ds.GetAssignedVPPApps(ctx, teamID)
if err != nil {
Expand Down Expand Up @@ -252,10 +294,24 @@ func (ds *Datastore) SetTeamVPPApps(ctx context.Context, teamID *uint, appFleets
}

for _, appFleet := range appFleets {
// upsert it if it does not exist or SelfService or InstallDuringSetup flags are changed
if existingFleet, ok := existingApps[appFleet.VPPAppID]; !ok || existingFleet.SelfService != appFleet.SelfService ||
// upsert it if it does not exist or labels or SelfService or InstallDuringSetup flags are changed
existingApp, isExistingApp := existingApps[appFleet.VPPAppID]
var labelsChanged bool
if isExistingApp {
existingLabels, err := ds.getExistingLabels(ctx, appFleet.AppTeamID)
if err != nil {
return ctxerr.Wrap(ctx, err, "getting existing labels for vpp app")
}

labelsChanged = !existingLabels.Equal(appFleet.ValidatedLabels)
}

if !isExistingApp ||
existingApp.SelfService != appFleet.SelfService ||
labelsChanged ||
appFleet.InstallDuringSetup != nil &&
existingFleet.InstallDuringSetup != nil && *appFleet.InstallDuringSetup != *existingFleet.InstallDuringSetup {
existingApp.InstallDuringSetup != nil &&
*appFleet.InstallDuringSetup != *existingApp.InstallDuringSetup {
toAddApps = append(toAddApps, appFleet)
}
}
Expand All @@ -270,9 +326,16 @@ func (ds *Datastore) SetTeamVPPApps(ctx context.Context, teamID *uint, appFleets

return ds.withRetryTxx(ctx, func(tx sqlx.ExtContext) error {
for _, toAdd := range toAddApps {
if _, err := insertVPPAppTeams(ctx, tx, toAdd, teamID, vppToken.ID); err != nil {
vppAppTeamID, err := insertVPPAppTeams(ctx, tx, toAdd, teamID, vppToken.ID)
if err != nil {
return ctxerr.Wrap(ctx, err, "SetTeamVPPApps inserting vpp app into team")
}

if toAdd.ValidatedLabels != nil {
if err := setOrUpdateSoftwareInstallerLabelsDB(ctx, tx, vppAppTeamID, *toAdd.ValidatedLabels, softwareTypeVPP); err != nil {
return ctxerr.Wrap(ctx, err, "failed to update labels on vpp apps batch operation")
}
}
}

for _, toRemove := range toRemoveApps {
Expand Down
Loading

0 comments on commit 053549f

Please sign in to comment.