diff --git a/changes/36759-gitops-team-labels b/changes/36759-gitops-team-labels new file mode 100644 index 0000000000..a2e0b24c74 --- /dev/null +++ b/changes/36759-gitops-team-labels @@ -0,0 +1 @@ +* Extended gitops to support team labels. \ No newline at end of file diff --git a/cmd/fleetctl/fleetctl/gitops.go b/cmd/fleetctl/fleetctl/gitops.go index 04a71e4cd2..f29b9c5226 100644 --- a/cmd/fleetctl/fleetctl/gitops.go +++ b/cmd/fleetctl/fleetctl/gitops.go @@ -143,25 +143,38 @@ func gitopsCommand() *cli.Command { secrets := make(map[string]struct{}) // We keep track of the environment FLEET_SECRET_* variables allFleetSecrets := make(map[string]string) - // Keep track of which labels we'd have after this gitops run. - var proposedLabelNames []string - // Get all labels ... this is used to both populate the proposedLabelNames list and check if - // we reference a built-in label (which is not allowed). - storedLabelNames := make(map[fleet.LabelType]map[string]interface{}) // label type -> label name set - // TODO gitops get labels for other teams instead of global-only - labels, err := fleetClient.GetLabels(0) + // We need the list of built-in labels for showing contextual errors in case the user + // decides to reference a built-in label. + builtInLabelNames := make(map[string]any) + globalLabels, err := fleetClient.GetLabels(0) if err != nil { - return fmt.Errorf("getting labels: %w", err) + return fmt.Errorf("getting global labels: %w", err) } - for _, lbl := range labels { - if storedLabelNames[lbl.LabelType] == nil { - storedLabelNames[lbl.LabelType] = make(map[string]interface{}) + for _, l := range globalLabels { + if l.LabelType == fleet.LabelTypeBuiltIn { + builtInLabelNames[l.Name] = nil } - storedLabelNames[lbl.LabelType][lbl.Name] = struct{}{} } - // Parsed config and filename pair + // We don't have access to the TeamID at this point in the time, and we need it down the pipeline to get + // the labels from existing teams + teamIDLookup := make(map[string]*uint) + teamIDLookup[spec.LabelAPIGlobalTeamName] = ptr.Uint(0) + if appConfig.License.IsPremium() { + teams, err := fleetClient.ListTeams("") + if err != nil { + return fmt.Errorf("getting teams: %w", err) + } + for _, tm := range teams { + teamIDLookup[tm.Name] = &tm.ID + } + } + + // Used for keeping track of all label changes in this run. + labelChanges := make(map[string][]spec.LabelChange) // team name -> label changes + + // Parsed a config and filename pair type ConfigFile struct { Config *spec.GitOps Filename string @@ -191,6 +204,11 @@ func gitopsCommand() *cli.Command { globalConfigLoaded = true } configFile := ConfigFile{Config: config, Filename: flFilename, IsGlobalConfig: isGlobalConfig} + + if !isGlobalConfig && !appConfig.License.IsPremium() { + logf("[!] skipping team config %s since teams are only supported for premium Fleet users\n", flFilename) + continue + } if isGlobalConfig { // If it's a global file, put it at the beginning // of the array so it gets processed first @@ -198,6 +216,33 @@ func gitopsCommand() *cli.Command { } else { configs = append(configs, configFile) } + + // We want to compute label changes early ... this will allow us to detect any monkey business around + // labels like trying to add the same label on different teams; labels can also move from one + // team to another, so we need the complete list of changes to plan the label movements. + teamName := config.CoercedTeamName() + teamID := teamIDLookup[teamName] + + if _, ok := labelChanges[teamName]; ok { + continue + } + + var existingLabels []*fleet.LabelSpec + if teamID != nil { + if *teamID == 0 { + existingLabels = globalLabels + } else { + if existingLabels, err = fleetClient.GetLabels(*teamID); err != nil { + return fmt.Errorf("getting team '%s' labels: %w", teamName, err) + } + } + } + labelChanges[teamName] = computeLabelChanges( + flFilename, + teamName, + existingLabels, + config.Labels, + ) } // fail if scripts are supplied on no-team and global config is missing @@ -205,6 +250,11 @@ func gitopsCommand() *cli.Command { return errors.New("global config must be provided alongside no-team.yml") } + labelMoves, err := computeLabelMoves(labelChanges) + if err != nil { + return err + } + for _, configFile := range configs { config := configFile.Config flFilename := configFile.Filename @@ -223,33 +273,6 @@ func gitopsCommand() *cli.Command { if !config.Controls.Set() { config.Controls = noTeamControls } - - // TODO GitOps move this to have team-specific and global names - - // If config.Labels is nil, it means we plan on deleting all existing labels. - if config.Labels == nil { - proposedLabelNames = make([]string, 0) - } else if len(config.Labels) > 0 { - // If config.Labels is populated, get the names it contains. - proposedLabelNames = make([]string, len(config.Labels)) - for i, l := range config.Labels { - proposedLabelNames[i] = l.Name - } - } - } else if !appConfig.License.IsPremium() { - logf("[!] skipping team config %s since teams are only supported for premium Fleet users\n", flFilename) - continue - } - - // If we haven't populated this list yet, it means we're either doing team-level GitOps only, - // or a global YAML was provided with no `labels:` key in it (meaning "keep existing labels"). - // In either case we'll get the list of label names from the db so we can ensure that we're - // not attempting to apply non-existent labels to other entities. - if proposedLabelNames == nil { - proposedLabelNames = make([]string, 0, len(storedLabelNames[fleet.LabelTypeRegular])) - for persistedLabel := range storedLabelNames[fleet.LabelTypeRegular] { - proposedLabelNames = append(proposedLabelNames, persistedLabel) - } } // Gather stats on where labels are used in this gitops config, @@ -261,26 +284,54 @@ func gitopsCommand() *cli.Command { return err } + // The validity of a label is based on their existence (either the label is going to be added or + // the label stayed the same). We look at both global label changes and team-specific label changes + // because of scoping rules (a team resource can reference a global label). + validLabelNames := make(map[string]struct{}) + if globalLabelChanges, ok := labelChanges[spec.LabelAPIGlobalTeamName]; ok { + for _, label := range globalLabelChanges { + if label.Op == "+" || label.Op == "=" { + validLabelNames[label.Name] = struct{}{} + } + } + } else { + // We are applying a stand-alone team config file, so no changes for the global labels were + // computed. + for _, l := range globalLabels { + if l.LabelType != fleet.LabelTypeBuiltIn { + validLabelNames[l.Name] = struct{}{} + } + } + } + if config.CoercedTeamName() != spec.LabelAPIGlobalTeamName { + for _, label := range labelChanges[config.CoercedTeamName()] { + if label.Op == "+" || label.Op == "=" { + validLabelNames[label.Name] = struct{}{} + } + } + } + // Check if any used labels are not in the proposed labels list. // If there are, we'll bail out with helpful error messages. unknownLabelsUsed := false builtInLabelsUsed := false for labelUsed := range labelsUsed { - if slices.Index(proposedLabelNames, labelUsed) == -1 { - if _, ok := storedLabelNames[fleet.LabelTypeBuiltIn][labelUsed]; ok { - logf( - "[!] '%s' label is built-in. Only custom labels are supported. If you want to target a specific platform please use 'platform' instead. If not, please create a custom label and try again. \n", - labelUsed, - ) - builtInLabelsUsed = true - } else { - for _, labelUser := range labelsUsed[labelUsed] { - logf("[!] Unknown label '%s' is referenced by %s '%s'\n", labelUsed, labelUser.Type, labelUser.Name) - } - unknownLabelsUsed = true - } + if _, ok := validLabelNames[labelUsed]; ok { + continue } + if _, ok := builtInLabelNames[labelUsed]; ok { + logf( + "[!] '%s' label is built-in. Only custom labels are supported. If you want to target a specific platform please use 'platform' instead. If not, please create a custom label and try again. \n", + labelUsed, + ) + builtInLabelsUsed = true + continue + } + for _, labelUsed := range labelsUsed[labelUsed] { + logf("[!] Unknown label '%s' is referenced by %s '%s'\n", labelUsed, labelUsed.Type, labelUsed.Name) + } + unknownLabelsUsed = true } if unknownLabelsUsed { return errors.New("Please create the missing labels, or update your settings to not refer to these labels.") @@ -289,11 +340,27 @@ func gitopsCommand() *cli.Command { return errors.New("Please update your settings to not refer to built-in labels.") } + teamName := config.CoercedTeamName() + labelChangesSummary := spec.NewLabelChangesSummary(labelChanges[teamName], labelMoves[teamName]) + config.LabelChangesSummary = labelChangesSummary + + // Delete labels at the end of the run to avoid issues with resource contention. + if !flDryRun { + for _, name := range labelChangesSummary.LabelsToRemove { + l := name // rebind for closure + allPostOps = append(allPostOps, func() error { + if err := fleetClient.DeleteLabel(l); err != nil { + return err + } + return nil + }) + } + } + // Special handling for tokens is required because they link to teams (by // name.) Because teams can be created/deleted during the same gitops run, we // grab some information to help us determine allowed/restricted actions and // when to perform the associations. - if isGlobalConfig && totalFilenames > 1 && !(totalFilenames == 2 && noTeamPresent) && appConfig.License.IsPremium() { abmTeams, hasMissingABMTeam, usesLegacyABMConfig, err = checkABMTeamAssignments(config, fleetClient) if err != nil { @@ -334,8 +401,8 @@ func gitopsCommand() *cli.Command { } } - // If team is not found, we need to remove the VPP config from - // the global config, and then apply it after teams are processed + // If a team is not found, we need to remove the VPP config from + // the global config and then apply it after teams are processed mdmMap["volume_purchasing_program"] = nil } } @@ -371,8 +438,19 @@ func gitopsCommand() *cli.Command { if err != nil { return err } - assumptions, postOps, err := fleetClient.DoGitOps(c.Context, config, flFilename, logf, flDryRun, teamDryRunAssumptions, appConfig, - teamsSoftwareInstallers, teamsVPPApps, teamsScripts, &iconSettings) + assumptions, err := fleetClient.DoGitOps( + c.Context, + config, + flFilename, + logf, + flDryRun, + teamDryRunAssumptions, + appConfig, + teamsSoftwareInstallers, + teamsVPPApps, + teamsScripts, + &iconSettings, + ) if err != nil { return err } @@ -381,7 +459,6 @@ func gitopsCommand() *cli.Command { } else { teamDryRunAssumptions = assumptions } - allPostOps = append(allPostOps, postOps...) } // if there were assignments to tokens, and some of the teams were missing at that time, submit a separate patch request to set them now. @@ -401,12 +478,22 @@ func gitopsCommand() *cli.Command { for _, teamWithApps := range missingVPPTeamsWithApps { _, _ = fmt.Fprintf(c.App.Writer, ReapplyingTeamForVPPAppsMsg, *teamWithApps.config.TeamName) teamWithApps.config.Software.AppStoreApps = teamWithApps.vppApps - _, postOps, err := fleetClient.DoGitOps(c.Context, teamWithApps.config, teamWithApps.filename, logf, flDryRun, teamDryRunAssumptions, appConfig, - teamsSoftwareInstallers, teamsVPPApps, teamsScripts, &iconSettings) + _, err := fleetClient.DoGitOps( + c.Context, + teamWithApps.config, + teamWithApps.filename, + logf, + flDryRun, + teamDryRunAssumptions, + appConfig, + teamsSoftwareInstallers, + teamsVPPApps, + teamsScripts, + &iconSettings, + ) if err != nil { return err } - allPostOps = append(allPostOps, postOps...) } if flDeleteOtherTeams && appConfig.License.IsPremium() { // skip team deletion for non-premium users @@ -443,18 +530,29 @@ func gitopsCommand() *cli.Command { if globalConfigLoaded && !noTeamPresent { defaultNoTeamConfig := new(spec.GitOps) defaultNoTeamConfig.TeamName = ptr.String(fleet.TeamNameNoTeam) - _, postOps, err := fleetClient.DoGitOps(c.Context, defaultNoTeamConfig, "no-team.yml", logf, flDryRun, nil, appConfig, - map[string][]fleet.SoftwarePackageResponse{}, map[string][]fleet.VPPAppResponse{}, map[string][]fleet.ScriptResponse{}, &iconSettings) + _, err := fleetClient.DoGitOps( + c.Context, + defaultNoTeamConfig, + "no-team.yml", + logf, + flDryRun, + nil, + appConfig, + map[string][]fleet.SoftwarePackageResponse{}, + map[string][]fleet.VPPAppResponse{}, + map[string][]fleet.ScriptResponse{}, + &iconSettings, + ) if err != nil { return err } - - allPostOps = append(allPostOps, postOps...) } - for _, postOp := range allPostOps { - if err := postOp(); err != nil { - return err + if !flDryRun { + for _, postOp := range allPostOps { + if err := postOp(); err != nil { + return err + } } } @@ -469,6 +567,108 @@ func gitopsCommand() *cli.Command { } } +// Computes label moves and validates that there is no funny business around label changes, +// like trying to add the same label on multiple teams or deleting the same label multiple times. +// A label is moved when it is deleted from one team and added to another, the moves are stored in +// a team name -> label names map. +func computeLabelMoves(allChanges map[string][]spec.LabelChange) (map[string][]spec.LabelMovement, error) { + deleteOps := make(map[string]spec.LabelChange) // label name -> file name + addOps := make(map[string]spec.LabelChange) // label name -> file name + + for _, teamChanges := range allChanges { + for _, change := range teamChanges { + switch change.Op { + case "-": + if prevCh, ok := deleteOps[change.Name]; ok { + errMsg := "can't delete label %q from %q, as it is already being deleted in %q" + return nil, fmt.Errorf(errMsg, change.Name, change.FileName, prevCh.FileName) + } + deleteOps[change.Name] = change + case "+": + if prevCh, ok := addOps[change.Name]; ok { + errMsg := "can't add label %q to %q, as it is already being added in %q" + return nil, fmt.Errorf(errMsg, change.Name, change.FileName, prevCh.FileName) + } + addOps[change.Name] = change + } + } + } + + // A label is moved if it is added ('+') to a team AND deleted ('-') from any other team + moves := make(map[string][]spec.LabelMovement) + for teamName, teamChanges := range allChanges { + for _, ch := range teamChanges { + if ch.Op == "+" { + if prevCh, isDeletedElsewhere := deleteOps[ch.Name]; isDeletedElsewhere { + moves[teamName] = append(moves[teamName], spec.LabelMovement{ + Name: ch.Name, + FromTeamName: prevCh.TeamName, + ToTeamName: ch.TeamName, + }) + } + } + } + } + + return moves, nil +} + +// Returns a list of label changes to be applied for either a global config file or a team config file. +func computeLabelChanges( + filename string, + teamName string, + existingLabels []*fleet.LabelSpec, + specifiedLabels []*fleet.LabelSpec, +) []spec.LabelChange { + var regularLabels []*fleet.LabelSpec + var labelOperations []spec.LabelChange + + for _, l := range existingLabels { + if l.LabelType == fleet.LabelTypeRegular { + regularLabels = append(regularLabels, l) + } + } + + // Handle the cases where the 'labels:' section is either nil (an empty 'labels:' section was specified, + // meaning remove-all) or an empty list (the 'labels:' section was not specified, so we do a no-op). + if len(specifiedLabels) == 0 { + op := "=" + if specifiedLabels == nil { + op = "-" + } + for _, l := range regularLabels { + change := spec.LabelChange{Name: l.Name, Op: op, TeamName: teamName, FileName: filename} + labelOperations = append(labelOperations, change) + } + return labelOperations + } + + specifiedMap := make(map[string]struct{}, len(specifiedLabels)) + for _, l := range specifiedLabels { + specifiedMap[l.Name] = struct{}{} + } + + // Determine which existing labels to remove. + for _, l := range regularLabels { + op := "-" + if _, ok := specifiedMap[l.Name]; ok { + op = "=" + } + // Remove from the map to track which specified labels are to be added. + delete(specifiedMap, l.Name) + change := spec.LabelChange{Name: l.Name, Op: op, TeamName: teamName, FileName: filename} + labelOperations = append(labelOperations, change) + } + + // Any names remaining in the map are new labels. + for lblName := range specifiedMap { + change := spec.LabelChange{Name: lblName, Op: "+", TeamName: teamName, FileName: filename} + labelOperations = append(labelOperations, change) + } + + return labelOperations +} + // Given a set of referenced labels and info about who is using them, update a provided usage map. func updateLabelUsage(labels []string, ident string, usageType string, currentUsage map[string][]LabelUsage) { for _, label := range labels { diff --git a/cmd/fleetctl/fleetctl/gitops_test.go b/cmd/fleetctl/fleetctl/gitops_test.go index 6f49236d2b..123ee81688 100644 --- a/cmd/fleetctl/fleetctl/gitops_test.go +++ b/cmd/fleetctl/fleetctl/gitops_test.go @@ -18,6 +18,7 @@ import ( "github.com/fleetdm/fleet/v4/cmd/fleetctl/fleetctl/testing_utils" "github.com/fleetdm/fleet/v4/pkg/file" "github.com/fleetdm/fleet/v4/pkg/optjson" + "github.com/fleetdm/fleet/v4/pkg/spec" "github.com/fleetdm/fleet/v4/server/config" "github.com/fleetdm/fleet/v4/server/datastore/mysql" "github.com/fleetdm/fleet/v4/server/fleet" @@ -295,6 +296,9 @@ func TestGitOpsBasicGlobalPremium(t *testing.T) { ds.ListQueriesFunc = func(ctx context.Context, opts fleet.ListQueryOptions) ([]*fleet.Query, int, *fleet.PaginationMetadata, error) { return nil, 0, nil, nil } + ds.ListTeamsFunc = func(ctx context.Context, filter fleet.TeamFilter, opt fleet.ListOptions) ([]*fleet.Team, error) { + return nil, nil + } // Mock DefaultTeamConfig functions for No Team webhook settings setupDefaultTeamConfigMocks(ds) @@ -646,6 +650,9 @@ func TestGitOpsBasicTeam(t *testing.T) { ds.DeleteIconsAssociatedWithTitlesWithoutInstallersFunc = func(ctx context.Context, teamID uint) error { return nil } + ds.ListTeamsFunc = func(ctx context.Context, filter fleet.TeamFilter, opt fleet.ListOptions) ([]*fleet.Team, error) { + return nil, nil + } // Mock DefaultTeamConfig functions for No Team webhook settings setupDefaultTeamConfigMocks(ds) @@ -1247,6 +1254,10 @@ func TestGitOpsFullTeam(t *testing.T) { return nil, nil } + ds.ListTeamsFunc = func(ctx context.Context, filter fleet.TeamFilter, opt fleet.ListOptions) ([]*fleet.Team, error) { + return nil, nil + } + // Mock DefaultTeamConfig functions for No Team webhook settings setupDefaultTeamConfigMocks(ds) @@ -4520,6 +4531,9 @@ func TestGitOpsWindowsUpdates(t *testing.T) { } return nil, ¬FoundError{} } + ds.ListTeamsFunc = func(ctx context.Context, filter fleet.TeamFilter, opt fleet.ListOptions) ([]*fleet.Team, error) { + return nil, nil + } // Track default team config for team 0 defaultTeamConfig := &fleet.TeamConfig{} @@ -4676,3 +4690,103 @@ software: assert.Empty(t, deleteCalls, "DeleteMDMWindowsConfigProfileByTeamAndName should not be called") }) } + +func TestComputeLabelChanges(t *testing.T) { + testCases := []struct { + name string + filename string + teamName string + existingLabels []*fleet.LabelSpec + specifiedLabels []*fleet.LabelSpec + expected []spec.LabelChange + }{ + { + name: "no specified labels removes all regular labels", + filename: "config.yml", + teamName: "team1", + existingLabels: []*fleet.LabelSpec{ + {Name: "label1", LabelType: fleet.LabelTypeRegular}, + {Name: "built-in", LabelType: fleet.LabelTypeBuiltIn}, + }, + specifiedLabels: nil, + expected: []spec.LabelChange{ + {Name: "label1", Op: "-", TeamName: "team1", FileName: "config.yml"}, + }, + }, + { + name: "empty list of specified labels is a no-op", + filename: "config.yml", + teamName: "team1", + existingLabels: []*fleet.LabelSpec{ + {Name: "label1", LabelType: fleet.LabelTypeRegular}, + }, + specifiedLabels: []*fleet.LabelSpec{}, + expected: []spec.LabelChange{ + {Name: "label1", Op: "=", TeamName: "team1", FileName: "config.yml"}, + }, + }, + { + name: "add, remove, and keep labels", + filename: "config.yml", + teamName: "team1", + existingLabels: []*fleet.LabelSpec{ + {Name: "to-remove", LabelType: fleet.LabelTypeRegular}, + {Name: "to-keep", LabelType: fleet.LabelTypeRegular}, + }, + specifiedLabels: []*fleet.LabelSpec{ + {Name: "to-keep"}, + {Name: "to-add"}, + }, + expected: []spec.LabelChange{ + {Name: "to-remove", Op: "-", TeamName: "team1", FileName: "config.yml"}, + {Name: "to-keep", Op: "=", TeamName: "team1", FileName: "config.yml"}, + {Name: "to-add", Op: "+", TeamName: "team1", FileName: "config.yml"}, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + changes := computeLabelChanges(tc.filename, tc.teamName, tc.existingLabels, tc.specifiedLabels) + require.ElementsMatch(t, tc.expected, changes) + }) + } +} + +func TestComputeLabelMoves(t *testing.T) { + t.Run("valid move between teams", func(t *testing.T) { + allChanges := map[string][]spec.LabelChange{ + "team1": { // Team 1 deletes "move-me" + {Name: "move-me", Op: "-", TeamName: "team1", FileName: "t1.yml"}, + }, + "team2": { // Team 2 adds "move-me" + {Name: "move-me", Op: "+", TeamName: "team2", FileName: "t2.yml"}, + }, + } + + moves, err := computeLabelMoves(allChanges) + require.NoError(t, err) + require.Len(t, moves["team2"], 1) + require.Equal(t, "move-me", moves["team2"][0].Name) + require.Equal(t, "team1", moves["team2"][0].FromTeamName) + require.Equal(t, "team2", moves["team2"][0].ToTeamName) + }) + + t.Run("conflict: duplicate delete", func(t *testing.T) { + allChanges := map[string][]spec.LabelChange{ + "team1": {{Name: "conflict", Op: "-", FileName: "file1.yml"}}, + "team2": {{Name: "conflict", Op: "-", FileName: "file2.yml"}}, + } + _, err := computeLabelMoves(allChanges) + require.ErrorContains(t, err, `already being deleted in "file1.yml"`) + }) + + t.Run("conflict: duplicate add", func(t *testing.T) { + allChanges := map[string][]spec.LabelChange{ + "team1": {{Name: "conflict", Op: "+", FileName: "file1.yml"}}, + "team2": {{Name: "conflict", Op: "+", FileName: "file2.yml"}}, + } + _, err := computeLabelMoves(allChanges) + require.ErrorContains(t, err, "already being added") + }) +} diff --git a/cmd/fleetctl/integrationtest/gitops/gitops_enterprise_integration_test.go b/cmd/fleetctl/integrationtest/gitops/gitops_enterprise_integration_test.go index ada5e254ca..3a6a8ea16f 100644 --- a/cmd/fleetctl/integrationtest/gitops/gitops_enterprise_integration_test.go +++ b/cmd/fleetctl/integrationtest/gitops/gitops_enterprise_integration_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "maps" "net/http" "net/http/httptest" "os" @@ -14,6 +15,7 @@ import ( "strings" "sync" "testing" + "text/template" "github.com/fleetdm/fleet/v4/cmd/fleetctl/fleetctl" "github.com/fleetdm/fleet/v4/cmd/fleetctl/fleetctl/testing_utils" @@ -39,6 +41,8 @@ import ( "github.com/stretchr/testify/suite" ) +const fleetGitopsRepo = "https://github.com/fleetdm/fleet-gitops" + func TestIntegrationsEnterpriseGitops(t *testing.T) { testingSuite := new(enterpriseIntegrationGitopsTestSuite) testingSuite.WithServer.Suite = &testingSuite.Suite @@ -161,6 +165,7 @@ func (s *enterpriseIntegrationGitopsTestSuite) TearDownTest() { func (s *enterpriseIntegrationGitopsTestSuite) assertDryRunOutput(t *testing.T, output string) { allowedVerbs := []string{ + "moved", "deleted", "updated", "applied", @@ -179,6 +184,7 @@ func (s *enterpriseIntegrationGitopsTestSuite) assertDryRunOutput(t *testing.T, func (s *enterpriseIntegrationGitopsTestSuite) assertRealRunOutput(t *testing.T, output string) { allowedVerbs := []string{ + "moving", "deleted", "updated", "applied", @@ -201,7 +207,6 @@ func (s *enterpriseIntegrationGitopsTestSuite) assertRealRunOutput(t *testing.T, // Changes to that repo may cause this test to fail. func (s *enterpriseIntegrationGitopsTestSuite) TestFleetGitops() { t := s.T() - const fleetGitopsRepo = "https://github.com/fleetdm/fleet-gitops" user := s.createGitOpsUser(t) fleetctlConfig := s.createFleetctlConfig(t, user) @@ -334,7 +339,7 @@ contexts: func (s *enterpriseIntegrationGitopsTestSuite) createGitOpsUser(t *testing.T) fleet.User { user := fleet.User{ - Name: "GitOps User", + Name: "GitOps User " + uuid.NewString(), Email: uuid.NewString() + "@example.com", GlobalRole: ptr.String(fleet.RoleGitOps), } @@ -2344,3 +2349,365 @@ team_settings: }) require.Equal(t, "Team Custom Ruby", teamDisplayName) } + +// TestGitOpsTeamLabels tests operations around team labels +func (s *enterpriseIntegrationGitopsTestSuite) TestGitOpsTeamLabels() { + t := s.T() + ctx := context.Background() + + user := s.createGitOpsUser(t) + fleetCfg := s.createFleetctlConfig(t, user) + + globalFile, err := os.CreateTemp(t.TempDir(), "*.yml") + require.NoError(t, err) + + // ----------------------------------------------------------------- + // First, let's validate that we can add labels to the global scope + // ----------------------------------------------------------------- + require.NoError(t, os.WriteFile(globalFile.Name(), []byte(` +agent_options: +controls: +org_settings: + secrets: + - secret: test_secret +policies: +queries: +labels: + - name: global-label-one + label_membership_type: dynamic + query: SELECT 1 + - name: global-label-two + label_membership_type: dynamic + query: SELECT 1 +`), 0o644)) + + s.assertDryRunOutput(t, fleetctl.RunAppForTest(t, []string{"gitops", "--config", fleetCfg.Name(), "-f", globalFile.Name(), "--dry-run"})) + s.assertRealRunOutput(t, fleetctl.RunAppForTest(t, []string{"gitops", "--config", fleetCfg.Name(), "-f", globalFile.Name()})) + + expected := make(map[string]uint) + expected["global-label-one"] = 0 + expected["global-label-two"] = 0 + + got := labelTeamIDResult(t, s, ctx) + + require.True(t, maps.Equal(expected, got)) + + // --------------------------------------------------------------- + // Now, let's validate that we can add and remove labels in a team + // --------------------------------------------------------------- + // TeamOne already exists + teamOneName := uuid.NewString() + teamOne, err := s.DS.NewTeam(context.Background(), &fleet.Team{Name: teamOneName}) + require.NoError(t, err) + + teamOneFile, err := os.CreateTemp(t.TempDir(), "*.yml") + require.NoError(t, err) + + require.NoError(t, os.WriteFile(teamOneFile.Name(), fmt.Appendf(nil, + ` +controls: +software: +queries: +policies: +agent_options: +name: %s +team_settings: + secrets: [{"secret":"enroll_secret"}] +labels: + - name: team-one-label-one + label_membership_type: dynamic + query: SELECT 2 + - name: team-one-label-two + label_membership_type: dynamic + query: SELECT 3 +`, teamOneName), 0o644)) + + s.assertDryRunOutput(t, fleetctl.RunAppForTest(t, []string{"gitops", "--config", fleetCfg.Name(), "-f", teamOneFile.Name(), "--dry-run"})) + s.assertRealRunOutput(t, fleetctl.RunAppForTest(t, []string{"gitops", "--config", fleetCfg.Name(), "-f", teamOneFile.Name()})) + + got = labelTeamIDResult(t, s, ctx) + + expected = make(map[string]uint) + expected["global-label-one"] = 0 + expected["global-label-two"] = 0 + expected["team-one-label-one"] = teamOne.ID + expected["team-one-label-two"] = teamOne.ID + + require.True(t, maps.Equal(expected, got)) + + // Try removing one label from teamOne + require.NoError(t, os.WriteFile(teamOneFile.Name(), fmt.Appendf(nil, + ` +controls: +software: +queries: +policies: +agent_options: +name: %s +team_settings: + secrets: [{"secret":"enroll_secret"}] +labels: + - name: team-one-label-one + label_membership_type: dynamic + query: SELECT 2 +`, teamOneName), 0o644)) + + s.assertRealRunOutput(t, fleetctl.RunAppForTest(t, []string{"gitops", "--config", fleetCfg.Name(), "-f", globalFile.Name(), "-f", teamOneFile.Name()})) + + expected = make(map[string]uint) + expected["global-label-one"] = 0 + expected["global-label-two"] = 0 + expected["team-one-label-one"] = teamOne.ID + + got = labelTeamIDResult(t, s, ctx) + + require.True(t, maps.Equal(expected, got)) + + // ------------------------------------------------ + // Finally, let's validate that we can move labels around + // ------------------------------------------------ + require.NoError(t, os.WriteFile(globalFile.Name(), []byte(` +agent_options: +controls: +org_settings: + secrets: + - secret: test_secret +policies: +queries: +labels: + - name: global-label-one + label_membership_type: dynamic + query: SELECT 1 + +`), 0o644)) + + require.NoError(t, os.WriteFile(teamOneFile.Name(), fmt.Appendf(nil, + + ` +controls: +software: +queries: +policies: +agent_options: +name: %s +team_settings: + secrets: [{"secret":"enroll_secret"}] +labels: + - name: team-one-label-two + label_membership_type: dynamic + query: SELECT 3 + - name: global-label-two + label_membership_type: dynamic + query: SELECT 1 +`, teamOneName), 0o644)) + + teamTwoName := uuid.NewString() + teamTwoFile, err := os.CreateTemp(t.TempDir(), "*.yml") + require.NoError(t, err) + + require.NoError(t, os.WriteFile(teamTwoFile.Name(), fmt.Appendf(nil, ` +controls: +software: +queries: +policies: +agent_options: +name: %s +team_settings: + secrets: [{"secret":"enroll_secret2"}] +labels: + - name: team-one-label-one + label_membership_type: dynamic + query: SELECT 2 +`, teamTwoName), 0o644)) + + s.assertDryRunOutput(t, fleetctl.RunAppForTest(t, []string{"gitops", "--config", fleetCfg.Name(), "-f", globalFile.Name(), "-f", teamOneFile.Name(), "-f", teamTwoFile.Name(), "--dry-run"})) + + // TODO: Seems like we require two passes to achieve equilibrium? + s.assertRealRunOutput(t, fleetctl.RunAppForTest(t, []string{"gitops", "--config", fleetCfg.Name(), "-f", globalFile.Name(), "-f", teamOneFile.Name(), "-f", teamTwoFile.Name()})) + s.assertRealRunOutput(t, fleetctl.RunAppForTest(t, []string{"gitops", "--config", fleetCfg.Name(), "-f", globalFile.Name(), "-f", teamOneFile.Name(), "-f", teamTwoFile.Name()})) + + teamTwo, err := s.DS.TeamByName(ctx, teamTwoName) + require.NoError(t, err) + + got = labelTeamIDResult(t, s, ctx) + + expected = make(map[string]uint) + expected["global-label-one"] = 0 + expected["team-one-label-two"] = teamOne.ID + expected["global-label-two"] = teamOne.ID + expected["team-one-label-one"] = teamTwo.ID + + require.True(t, maps.Equal(expected, got)) +} + +// Tests a gitops setup where every team runs from an independent repo. Multiple repos are simulated by +// copying over the example repository multiple times. +func (s *enterpriseIntegrationGitopsTestSuite) TestGitOpsTeamLabelsMultipleRepos() { + t := s.T() + ctx := context.Background() + + var users []fleet.User + var cfgPaths []*os.File + var reposDir []string + + for range 2 { + user := s.createGitOpsUser(t) + users = append(users, user) + + cfg := s.createFleetctlConfig(t, user) + cfgPaths = append(cfgPaths, cfg) + + repoDir := t.TempDir() + _, err := git.PlainClone( + repoDir, false, &git.CloneOptions{ + ReferenceName: "main", + SingleBranch: true, + Depth: 1, + URL: fleetGitopsRepo, + Progress: os.Stdout, + }, + ) + require.NoError(t, err) + reposDir = append(reposDir, repoDir) + } + + // Set the required environment variables + t.Setenv("FLEET_URL", s.Server.URL) + t.Setenv("FLEET_GLOBAL_ENROLL_SECRET", "global_enroll_secret") + t.Setenv("FLEET_WORKSTATIONS_ENROLL_SECRET", "workstations_enroll_secret") + t.Setenv("FLEET_WORKSTATIONS_CANARY_ENROLL_SECRET", "workstations_canary_enroll_secret") + + type tmplParams struct { + Name string + Queries string + Labels string + } + teamCfgTmpl, err := template.New("t1").Parse(` +controls: +software: +queries:{{ .Queries }} +policies: +labels:{{ .Labels }} +agent_options: +name:{{ .Name }} +team_settings: + secrets: [{"secret":"{{ .Name}}_secret"}] +`) + require.NoError(t, err) + + // -------------------------------------------------- + // First, lets simulate adding a new team per repo + // -------------------------------------------------- + for i, repo := range reposDir { + globalFile := path.Join(repo, "default.yml") + + newTeamCfgFile, err := os.CreateTemp(t.TempDir(), "*.yml") + require.NoError(t, err) + + require.NoError(t, teamCfgTmpl.Execute(newTeamCfgFile, tmplParams{ + Name: fmt.Sprintf(" team-%d", i), + Queries: fmt.Sprintf("\n - name: query-%d\n query: SELECT 1", i), + Labels: fmt.Sprintf("\n - name: label-%d\n label_membership_type: dynamic\n query: SELECT 1", i), + })) + + args := []string{"gitops", "--config", cfgPaths[i].Name(), "-f", globalFile, "-f", newTeamCfgFile.Name()} + s.assertRealRunOutput(t, fleetctl.RunAppForTest(t, args)) + } + + for i, user := range users { + team, err := s.DS.TeamByName(ctx, fmt.Sprintf("team-%d", i)) + require.NoError(t, err) + require.NotNil(t, team) + + queries, _, _, err := s.DS.ListQueries(ctx, fleet.ListQueryOptions{TeamID: &team.ID}) + require.NoError(t, err) + require.Len(t, queries, 1) + require.Equal(t, fmt.Sprintf("query-%d", i), queries[0].Name) + require.Equal(t, "SELECT 1", queries[0].Query) + require.NotNil(t, queries[0].TeamID) + require.Equal(t, *queries[0].TeamID, team.ID) + require.NotNil(t, queries[0].AuthorID) + require.Equal(t, *queries[0].AuthorID, user.ID) + + label, err := s.DS.LabelByName(ctx, fmt.Sprintf("label-%d", i), fleet.TeamFilter{User: &fleet.User{ID: user.ID}}) + require.NoError(t, err) + require.NotNil(t, label) + require.NotNil(t, label.TeamID) + require.Equal(t, *label.TeamID, team.ID) + require.NotNil(t, label.AuthorID) + require.Equal(t, *label.AuthorID, user.ID) + } + + // ----------------------------------------------------------------- + // Then, lets simulate a mutation by dropping the labels on team one + // ----------------------------------------------------------------- + for i, repo := range reposDir { + globalFile := path.Join(repo, "default.yml") + + newTeamCfgFile, err := os.CreateTemp(t.TempDir(), "*.yml") + require.NoError(t, err) + + params := tmplParams{ + Name: fmt.Sprintf(" team-%d", i), + Queries: fmt.Sprintf("\n - name: query-%d\n query: SELECT 1", i), + } + if i != 0 { + params.Labels = fmt.Sprintf("\n - name: label-%d\n label_membership_type: dynamic\n query: SELECT 1", i) + } + + require.NoError(t, teamCfgTmpl.Execute(newTeamCfgFile, params)) + + args := []string{"gitops", "--config", cfgPaths[i].Name(), "-f", globalFile, "-f", newTeamCfgFile.Name()} + s.assertRealRunOutput(t, fleetctl.RunAppForTest(t, args)) + } + + for i, user := range users { + team, err := s.DS.TeamByName(ctx, fmt.Sprintf("team-%d", i)) + require.NoError(t, err) + require.NotNil(t, team) + + queries, _, _, err := s.DS.ListQueries(ctx, fleet.ListQueryOptions{TeamID: &team.ID}) + require.NoError(t, err) + require.Len(t, queries, 1) + require.Equal(t, fmt.Sprintf("query-%d", i), queries[0].Name) + require.Equal(t, "SELECT 1", queries[0].Query) + require.NotNil(t, queries[0].TeamID) + require.Equal(t, *queries[0].TeamID, team.ID) + require.NotNil(t, queries[0].AuthorID) + require.Equal(t, *queries[0].AuthorID, user.ID) + + label, err := s.DS.LabelByName(ctx, fmt.Sprintf("label-%d", i), fleet.TeamFilter{User: &fleet.User{ID: user.ID}}) + if i == 0 { + require.Error(t, err) + require.Nil(t, label) + } else { + require.NoError(t, err) + require.NotNil(t, label) + require.NotNil(t, label.TeamID) + require.Equal(t, *label.TeamID, team.ID) + require.NotNil(t, label.AuthorID) + require.Equal(t, *label.AuthorID, user.ID) + } + } +} + +func labelTeamIDResult(t *testing.T, s *enterpriseIntegrationGitopsTestSuite, ctx context.Context) map[string]uint { + type labelResult struct { + Name string `db:"name"` + TeamID *uint `db:"team_id"` + } + var result []labelResult + mysql.ExecAdhocSQL(t, s.DS, func(q sqlx.ExtContext) error { + require.NoError(t, sqlx.SelectContext(ctx, q, &result, "SELECT name, team_id FROM labels WHERE label_type = 0")) + return nil + }) + got := make(map[string]uint) + for _, r := range result { + var teamID uint + if r.TeamID != nil { + teamID = *r.TeamID + } + got[r.Name] = teamID + } + return got +} diff --git a/pkg/spec/gitops.go b/pkg/spec/gitops.go index 78d0bcca6c..5d16e315ff 100644 --- a/pkg/spec/gitops.go +++ b/pkg/spec/gitops.go @@ -19,6 +19,58 @@ import ( "golang.org/x/text/unicode/norm" ) +const LabelAPIGlobalTeamName = "global" + +// LabelChangesSummary carries extra context of the labels operations for a config. +type LabelChangesSummary struct { + LabelsToUpdate []string + LabelsToAdd []string + LabelsToRemove []string + LabelsMovements []LabelMovement +} + +func NewLabelChangesSummary(changes []LabelChange, moves []LabelMovement) LabelChangesSummary { + r := LabelChangesSummary{ + LabelsMovements: moves, + } + + lookUp := make(map[string]any) + for _, m := range moves { + lookUp[m.Name] = nil + } + + for _, change := range changes { + if _, ok := lookUp[change.Name]; ok { + continue + } + switch change.Op { + case "+": + + r.LabelsToAdd = append(r.LabelsToAdd, change.Name) + case "-": + r.LabelsToRemove = append(r.LabelsToRemove, change.Name) + case "=": + r.LabelsToUpdate = append(r.LabelsToUpdate, change.Name) + } + } + return r +} + +// LabelMovement specifies a label movement, a label is moved if its removed from one team and added to another +type LabelMovement struct { + FromTeamName string // Source team name + ToTeamName string // Dest. team name + Name string // The globally unique label name +} + +// LabelChange used for keeping track of label operations +type LabelChange struct { + Name string // The globally unique label name + Op string // What operation to perform on the label. +:add, -:remove, =:no-op + TeamName string // The team this label belongs to. + FileName string // The filename that contains the label change +} + type ParseTypeError struct { Filename string // The name of the file being parsed Keys []string // The complete path to the field @@ -196,7 +248,10 @@ type GitOps struct { Controls GitOpsControls Policies []*GitOpsPolicySpec Queries []*fleet.QuerySpec - Labels []*fleet.LabelSpec + + Labels []*fleet.LabelSpec + LabelChangesSummary LabelChangesSummary + // Software is only allowed on teams, not on global config. Software GitOpsSoftware // FleetSecrets is a map of secret names to their values, extracted from FLEET_SECRET_ environment variables used in profiles and scripts. @@ -275,11 +330,7 @@ func GitOpsFromFile(filePath, baseDir string, appConfig *fleet.EnrichedAppConfig // Get the labels. If `labels:` is specified but no labels are listed, this will // set Labels as nil. If `labels:` isn't present at all, it will be set as an // empty array. - _, ok := top["labels"] - if !ok || !result.IsGlobal() { - if ok && !result.IsGlobal() { - logFn("[!] 'labels' is only supported in global settings. This key will be ignored.\n") - } + if _, ok := top["labels"]; !ok { result.Labels = make([]*fleet.LabelSpec, 0) } else { multiError = parseLabels(top, result, baseDir, filePath, multiError) @@ -328,6 +379,13 @@ func isNoTeam(teamName string) bool { return strings.EqualFold(teamName, noTeam) } +func (g *GitOps) CoercedTeamName() string { + if g.global() { + return LabelAPIGlobalTeamName + } + return *g.TeamName +} + const noTeam = "No team" func parseOrgSettings(raw json.RawMessage, result *GitOps, baseDir string, filePath string, multiError *multierror.Error) *multierror.Error { diff --git a/server/service/client.go b/server/service/client.go index 70b5673f02..6785a99970 100644 --- a/server/service/client.go +++ b/server/service/client.go @@ -12,7 +12,6 @@ import ( "net/http" "os" "path/filepath" - "slices" "strconv" "strings" "time" @@ -559,7 +558,7 @@ func (c *Client) ApplyGroup( return nil, nil, nil, nil, errors.New("Cannot import built-in labels. Please remove labels with a label_type of builtin and try again.") } } - if err := c.ApplyLabels(specs.Labels); err != nil { + if err := c.ApplyLabels(specs.Labels, nil, nil); err != nil { return nil, nil, nil, nil, fmt.Errorf("applying labels: %w", err) } logfn(appliedFormat, numberWithPluralization(len(specs.Labels), "label", "labels")) @@ -1836,7 +1835,7 @@ func (c *Client) DoGitOps( teamsVPPApps map[string][]fleet.VPPAppResponse, teamsScripts map[string][]fleet.ScriptResponse, iconSettings *fleet.IconGitOpsSettings, -) (*fleet.TeamSpecsDryRunAssumptions, []func() error, error) { +) (*fleet.TeamSpecsDryRunAssumptions, error) { baseDir := filepath.Dir(fullFilename) filename := filepath.Base(fullFilename) var teamAssumptions *fleet.TeamSpecsDryRunAssumptions @@ -1853,8 +1852,6 @@ func (c *Client) DoGitOps( var mdmAppConfig map[string]interface{} var team map[string]interface{} - var postOps []func() error - var eulaPath string group := spec.Group{} // as we parse the incoming gitops spec, we'll build out various group specs that will each be applied separately @@ -1876,27 +1873,17 @@ func (c *Client) DoGitOps( // OrgSettings so that they are not applied as part of the AppConfig. groupedCAs, err := fleet.ValidateCertificateAuthoritiesSpec(incoming.OrgSettings["certificate_authorities"]) if err != nil { - return nil, nil, fmt.Errorf("invalid certificate_authorities: %w", err) + return nil, fmt.Errorf("invalid certificate_authorities: %w", err) } group.CertificateAuthorities = groupedCAs delete(incoming.OrgSettings, "certificate_authorities") // Labels - // TODO GitOps if incoming.Labels == nil || len(incoming.Labels) > 0 { - labelsToDelete, err := c.doGitOpsLabels(incoming, logFn, dryRun) + err := c.doGitOpsLabels(incoming, logFn, dryRun) if err != nil { - return nil, nil, err + return nil, err } - postOps = append(postOps, func() error { - for _, labelToDelete := range labelsToDelete { - logFn("[-] deleting label '%s'\n", labelToDelete) - if err := c.DeleteLabel(labelToDelete); err != nil { - return err - } - } - return nil - }) } // Features @@ -1908,7 +1895,7 @@ func (c *Client) DoGitOps( } features, ok = features.(map[string]any) if !ok { - return nil, nil, errors.New("org_settings.features config is not a map") + return nil, errors.New("org_settings.features config is not a map") } if enableSoftwareInventory, ok := features.(map[string]any)["enable_software_inventory"]; !ok || enableSoftwareInventory == nil { features.(map[string]any)["enable_software_inventory"] = true @@ -1922,7 +1909,7 @@ func (c *Client) DoGitOps( } integrations, ok = integrations.(map[string]interface{}) if !ok { - return nil, nil, errors.New("org_settings.integrations config is not a map") + return nil, errors.New("org_settings.integrations config is not a map") } if jira, ok := integrations.(map[string]interface{})["jira"]; !ok || jira == nil { integrations.(map[string]interface{})["jira"] = []interface{}{} @@ -1938,13 +1925,13 @@ func (c *Client) DoGitOps( } // ensure that legacy certificate authorities are not set in integrations if _, ok := integrations.(map[string]interface{})["ndes_scep_proxy"]; ok { - return nil, nil, errors.New("org_settings.integrations.ndes_scep_proxy is not supported, please use org_settings.certificate_authorities.ndes_scep_proxy instead") + return nil, errors.New("org_settings.integrations.ndes_scep_proxy is not supported, please use org_settings.certificate_authorities.ndes_scep_proxy instead") } if _, ok := integrations.(map[string]interface{})["digicert"]; ok { - return nil, nil, errors.New("org_settings.integrations.digicert is not supported, please use org_settings.certificate_authorities.digicert instead") + return nil, errors.New("org_settings.integrations.digicert is not supported, please use org_settings.certificate_authorities.digicert instead") } if _, ok := integrations.(map[string]interface{})["custom_scep_proxy"]; ok { - return nil, nil, errors.New("org_settings.integrations.custom_scep_proxy is not supported, please use org_settings.certificate_authorities.custom_scep_proxy instead") + return nil, errors.New("org_settings.integrations.custom_scep_proxy is not supported, please use org_settings.certificate_authorities.custom_scep_proxy instead") } // Ensure webhooks settings exists @@ -1999,7 +1986,7 @@ func (c *Client) DoGitOps( } mdmAppConfig, ok = mdmConfig.(map[string]interface{}) if !ok { - return nil, nil, errors.New("org_settings.mdm config is not a map") + return nil, errors.New("org_settings.mdm config is not a map") } if _, ok := mdmAppConfig["apple_bm_default_team"]; !ok && appConfig.License.IsPremium() { @@ -2082,7 +2069,7 @@ func (c *Client) DoGitOps( } err = c.doGitOpsEULA(eulaPath, logFn, dryRun) if err != nil { - return nil, nil, err + return nil, err } } @@ -2137,7 +2124,7 @@ func (c *Client) DoGitOps( } features, ok = features.(map[string]any) if !ok { - return nil, nil, fmt.Errorf("Team %s features config is not a map", *incoming.TeamName) + return nil, fmt.Errorf("Team %s features config is not a map", *incoming.TeamName) } if enableSoftwareInventory, ok := features.(map[string]any)["enable_software_inventory"]; !ok || enableSoftwareInventory == nil { features.(map[string]any)["enable_software_inventory"] = true @@ -2151,7 +2138,7 @@ func (c *Client) DoGitOps( team["integrations"] = integrations _, ok = integrations.(map[string]interface{}) if !ok { - return nil, nil, errors.New("team_settings.integrations config is not a map") + return nil, errors.New("team_settings.integrations config is not a map") } if googleCal, ok := integrations.(map[string]interface{})["google_calendar"]; !ok || googleCal == nil { @@ -2159,7 +2146,7 @@ func (c *Client) DoGitOps( } else { _, ok = googleCal.(map[string]interface{}) if !ok { - return nil, nil, errors.New("team_settings.integrations.google_calendar config is not a map") + return nil, errors.New("team_settings.integrations.google_calendar config is not a map") } } @@ -2168,7 +2155,7 @@ func (c *Client) DoGitOps( } else { _, ok = conditionalAccessEnabled.(bool) if !ok { - return nil, nil, errors.New("team_settings.integrations.conditional_access_enabled config is not a bool") + return nil, errors.New("team_settings.integrations.conditional_access_enabled config is not a bool") } } @@ -2177,6 +2164,7 @@ func (c *Client) DoGitOps( } if !incoming.IsNoTeam() { + // Common controls settings between org and team settings // Put in default values for macos_settings if incoming.Controls.MacOSSettings != nil { @@ -2288,7 +2276,7 @@ func (c *Client) DoGitOps( requireBitLockerPIN = incoming.Controls.RequireBitLockerPIN.(bool) } if !enableDiskEncryption && requireBitLockerPIN { - return nil, nil, errors.New("enable_disk_encryption cannot be false if windows_require_bitlocker_pin is true") + return nil, errors.New("enable_disk_encryption cannot be false if windows_require_bitlocker_pin is true") } mdmAppConfig["enable_disk_encryption"] = enableDiskEncryption @@ -2298,7 +2286,7 @@ func (c *Client) DoGitOps( team["gitops_filename"] = filename rawTeam, err := json.Marshal(team) if err != nil { - return nil, nil, fmt.Errorf("error marshalling team spec: %w", err) + return nil, fmt.Errorf("error marshalling team spec: %w", err) } group.Teams = []json.RawMessage{rawTeam} group.TeamsDryRunAssumptions = teamDryRunAssumptions @@ -2314,7 +2302,7 @@ func (c *Client) DoGitOps( ExpandEnvConfigProfiles: true, }, teamsSoftwareInstallers, teamsVPPApps, teamsScripts) if err != nil { - return nil, nil, err + return nil, err } var teamSoftwareInstallers []fleet.SoftwarePackageResponse @@ -2324,7 +2312,7 @@ func (c *Client) DoGitOps( if incoming.TeamName != nil { if !incoming.IsNoTeam() { if len(teamIDsByName) != 1 { - return nil, nil, fmt.Errorf("expected 1 team spec to be applied, got %d", len(teamIDsByName)) + return nil, fmt.Errorf("expected 1 team spec to be applied, got %d", len(teamIDsByName)) } teamID, ok := teamIDsByName[*incoming.TeamName] if ok && teamID == 0 { @@ -2345,24 +2333,33 @@ func (c *Client) DoGitOps( *incoming.TeamName) } - return nil, postOps, nil + return nil, nil } - return nil, nil, fmt.Errorf("team %s not created", *incoming.TeamName) + return nil, fmt.Errorf("team %s not created", *incoming.TeamName) } for _, teamID = range teamIDsByName { incoming.TeamID = &teamID } + + // Apply team labels after any possible new teams are created + if incoming.Labels == nil || len(incoming.Labels) > 0 { + err := c.doGitOpsLabels(incoming, logFn, dryRun) + if err != nil { + return nil, err + } + } + teamSoftwareInstallers = teamsSoftwareInstallers[*incoming.TeamName] teamVPPApps = teamsVPPApps[*incoming.TeamName] teamScripts = teamsScripts[*incoming.TeamName] } else { noTeamSoftwareInstallers, noTeamVPPApps, err := c.doGitOpsNoTeamSetupAndSoftware(incoming, baseDir, appConfig, logFn, dryRun) if err != nil { - return nil, nil, err + return nil, err } // Apply webhook settings for "No Team" if err := c.doGitOpsNoTeamWebhookSettings(incoming, appConfig, logFn, dryRun); err != nil { - return nil, nil, fmt.Errorf("applying no team webhook settings: %w", err) + return nil, fmt.Errorf("applying no team webhook settings: %w", err) } teamSoftwareInstallers = noTeamSoftwareInstallers teamVPPApps = noTeamVPPApps @@ -2372,7 +2369,7 @@ func (c *Client) DoGitOps( err = c.doGitOpsPolicies(incoming, teamSoftwareInstallers, teamVPPApps, teamScripts, logFn, dryRun) if err != nil { - return nil, nil, err + return nil, err } // Apply Android certificates if present @@ -2380,9 +2377,9 @@ func (c *Client) DoGitOps( if err != nil { var gitOpsErr *gitOpsValidationError if errors.As(err, &gitOpsErr) { - return nil, nil, gitOpsErr.WithFileContext(baseDir, filename) + return nil, gitOpsErr.WithFileContext(baseDir, filename) } - return nil, nil, err + return nil, err } // apply icon changes from software installers and VPP apps @@ -2396,7 +2393,7 @@ func (c *Client) DoGitOps( ) } else { if err = c.doGitOpsIcons(iconUpdates, iconSettings.ConcurrentUploads, iconSettings.ConcurrentUpdates); err != nil { - return nil, nil, err + return nil, err } logFn( "[+] set icons on %s and deleted icons on %s\n", @@ -2412,11 +2409,11 @@ func (c *Client) DoGitOps( if !incoming.IsNoTeam() { err = c.doGitOpsQueries(incoming, logFn, dryRun) if err != nil { - return nil, nil, err + return nil, err } } - return teamAssumptions, postOps, nil + return teamAssumptions, nil } func (c *Client) doGitOpsIcons(iconUpdates fleet.IconChanges, concurrentUploads int, concurrentUpdates int) error { @@ -2666,49 +2663,42 @@ func (c *Client) doGitOpsNoTeamWebhookSettings( return nil } -// TODO allow spec'ing labels by either "everything" or team-specific (team ID or name?) -func (c *Client) doGitOpsLabels(config *spec.GitOps, logFn func(format string, args ...interface{}), dryRun bool) ([]string, error) { - persistedLabels, err := c.GetLabels(0) // TODO handle per-team - if err != nil { - return nil, err - } - var numUpdates int - var labelsToDelete []string - for _, persistedLabel := range persistedLabels { - if persistedLabel.LabelType == fleet.LabelTypeBuiltIn { - continue - } - if slices.IndexFunc(config.Labels, func(configLabel *fleet.LabelSpec) bool { return configLabel.Name == persistedLabel.Name }) == -1 { - labelsToDelete = append(labelsToDelete, persistedLabel.Name) - } else { - numUpdates++ - } - } - numNew := len(config.Labels) - numUpdates +func (c *Client) doGitOpsLabels( + config *spec.GitOps, + logFn func(format string, args ...any), + dryRun bool, +) error { + toDelete := config.LabelChangesSummary.LabelsToRemove + toMove := config.LabelChangesSummary.LabelsMovements + nToAdd := len(config.LabelChangesSummary.LabelsToAdd) + nToUpdate := len(config.LabelChangesSummary.LabelsToUpdate) + if dryRun { - for _, labelToDelete := range labelsToDelete { + for _, labelToDelete := range toDelete { logFn("[-] would've deleted label '%s'\n", labelToDelete) } - if numNew > 0 { - logFn("[+] would've created %s\n", numberWithPluralization(numNew, "label", "labels")) + if nToAdd > 0 { + logFn("[+] would've created %s\n", numberWithPluralization(nToAdd, "label", "labels")) } - if numUpdates > 0 { - logFn("[+] would've updated %s\n", numberWithPluralization(numUpdates, "label", "labels")) + if nToUpdate > 0 { + logFn("[+] would've updated %s\n", numberWithPluralization(nToUpdate, "label", "labels")) } - return nil, nil + for _, l := range toMove { + logFn("[-] would've moved label %q from team %q to team %q\n", l.Name, l.FromTeamName, l.ToTeamName) + } + return nil } - if dryRun { - logFn("[+] would've applied %s (%d new and %d updated)\n", numberWithPluralization(len(config.Labels), "label", "labels"), len(config.Labels)-numUpdates, numUpdates) - } else { - logFn("[+] applying %s (%d new and %d updated)\n", numberWithPluralization(len(config.Labels), "label", "labels"), len(config.Labels)-numUpdates, numUpdates) + for _, l := range toDelete { + logFn("[-] deleting label '%s'\n", l) } - - err = c.ApplyLabels(config.Labels) - if err != nil { - return nil, err + namesToMove := make([]string, 0, len(toMove)) + for _, l := range toMove { + namesToMove = append(namesToMove, l.Name) + logFn("[-] moving label %q from team %q to team %q\n", l.Name, l.FromTeamName, l.ToTeamName) } - return labelsToDelete, nil + logFn("[+] applying %s (%d new and %d updated)\n", numberWithPluralization(len(config.Labels), "label", "labels"), nToAdd, nToUpdate) + return c.ApplyLabels(config.Labels, config.TeamID, namesToMove) } func (c *Client) doGitOpsPolicies(config *spec.GitOps, teamSoftwareInstallers []fleet.SoftwarePackageResponse, teamVPPApps []fleet.VPPAppResponse, teamScripts []fleet.ScriptResponse, logFn func(format string, args ...interface{}), dryRun bool) error { diff --git a/server/service/client_labels.go b/server/service/client_labels.go index 91135537a5..e937047762 100644 --- a/server/service/client_labels.go +++ b/server/service/client_labels.go @@ -8,12 +8,25 @@ import ( ) // ApplyLabels sends the list of Labels to be applied (upserted) to the -// Fleet instance. -// TODO gitops allow specifying by team -func (c *Client) ApplyLabels(specs []*fleet.LabelSpec) error { - req := applyLabelSpecsRequest{Specs: specs} +// Fleet instance. Use teamID = nil for global labels. +func (c *Client) ApplyLabels( + specs []*fleet.LabelSpec, + teamID *uint, + moves []string, +) error { + req := applyLabelSpecsRequest{TeamID: teamID, Specs: specs, NamesToMove: moves} verb, path := "POST", "/api/latest/fleet/spec/labels" var responseBody applyLabelSpecsResponse + + if teamID != nil { + return c.authenticatedRequestWithQuery( + req, + verb, + path, + &responseBody, + fmt.Sprintf("team_id=%d", *teamID), + ) + } return c.authenticatedRequest(req, verb, path, &responseBody) } diff --git a/server/service/client_test.go b/server/service/client_test.go index ec2e498908..4d519786d7 100644 --- a/server/service/client_test.go +++ b/server/service/client_test.go @@ -1042,7 +1042,7 @@ func TestGitOpsErrors(t *testing.T) { require.NoError(t, err) config.OrgSettings["secrets"] = []*fleet.EnrollSecret{} settings := fleet.IconGitOpsSettings{ConcurrentUpdates: 1, ConcurrentUploads: 1} - _, _, err = client.DoGitOps(ctx, config, "/filename", nil, false, nil, nil, nil, nil, nil, &settings) + _, err = client.DoGitOps(ctx, config, "/filename", nil, false, nil, nil, nil, nil, nil, &settings) assert.ErrorContains(t, err, tt.wantErr) }) } diff --git a/tools/loadtest/fleetd_labels/main.go b/tools/loadtest/fleetd_labels/main.go index 6f00f767ff..5dad66cddd 100644 --- a/tools/loadtest/fleetd_labels/main.go +++ b/tools/loadtest/fleetd_labels/main.go @@ -137,7 +137,7 @@ func main() { labelSpecSubset := *labelSpec labelSpecSubset.Hosts = batch printf("Applying label %s to %d hosts...\n", labelSpecSubset.Name, len(labelSpecSubset.Hosts)) - if err := apiClient.ApplyLabels([]*fleet.LabelSpec{&labelSpecSubset}); err != nil { + if err := apiClient.ApplyLabels([]*fleet.LabelSpec{&labelSpecSubset}, nil, nil); err != nil { panic(err) } printf("Applied label %s to %d hosts\n", labelSpecSubset.Name, len(labelSpecSubset.Hosts))