From c5bffe7c0dad59d01304ba0a7663cfdead2f87e6 Mon Sep 17 00:00:00 2001 From: Dante Catalfamo <43040593+dantecatalfamo@users.noreply.github.com> Date: Tue, 18 Mar 2025 12:24:35 -0400 Subject: [PATCH] Allow team gitops to run without global config (#26969) #26171 --- changes/26171-gitops-per-team | 1 + cmd/fleetctl/gitops.go | 48 ++++++++++++++++++++++---------- cmd/fleetctl/gitops_test.go | 10 +++++-- ee/server/service/mdm_test.go | 2 +- server/authz/policy.rego | 4 +-- server/authz/policy_test.go | 2 +- server/service/appconfig_test.go | 4 +-- 7 files changed, 47 insertions(+), 24 deletions(-) create mode 100644 changes/26171-gitops-per-team diff --git a/changes/26171-gitops-per-team b/changes/26171-gitops-per-team new file mode 100644 index 0000000000..7028074ba6 --- /dev/null +++ b/changes/26171-gitops-per-team @@ -0,0 +1 @@ +- Allow team gitops to run without global config diff --git a/cmd/fleetctl/gitops.go b/cmd/fleetctl/gitops.go index 20fec79941..cf79d91f86 100644 --- a/cmd/fleetctl/gitops.go +++ b/cmd/fleetctl/gitops.go @@ -9,7 +9,6 @@ import ( "github.com/fleetdm/fleet/v4/pkg/spec" "github.com/fleetdm/fleet/v4/server/fleet" - "github.com/fleetdm/fleet/v4/server/ptr" "github.com/fleetdm/fleet/v4/server/service" "github.com/urfave/cli/v2" "golang.org/x/text/unicode/norm" @@ -33,7 +32,7 @@ func gitopsCommand() *cli.Command { Required: true, EnvVars: []string{"FILENAME"}, Destination: &flFilenames, - Usage: "The file(s) with the GitOps configuration. If multiple files are provided, the first file must be the global configuration and the rest must be team configurations.", + Usage: "The file(s) with the GitOps configuration.", }, &cli.BoolFlag{ Name: "delete-other-teams", @@ -90,13 +89,9 @@ func gitopsCommand() *cli.Command { var originalABMConfig []any var originalVPPConfig []any var teamNames []string - var firstFileMustBeGlobal *bool var teamDryRunAssumptions *fleet.TeamSpecsDryRunAssumptions var abmTeams, vppTeams []string var hasMissingABMTeam, hasMissingVPPTeam, usesLegacyABMConfig bool - if totalFilenames > 1 { - firstFileMustBeGlobal = ptr.Bool(true) - } // we keep track of team software installers and scripts for correct policy application teamsSoftwareInstallers := make(map[string][]fleet.SoftwarePackageResponse) @@ -107,6 +102,20 @@ func gitopsCommand() *cli.Command { secrets := make(map[string]struct{}) // We keep track of the environment FLEET_SECRET_* variables allFleetSecrets := make(map[string]string) + + // Parsed config and filename pair + type ConfigFile struct { + Config *spec.GitOps + Filename string + IsGlobalConfig bool + } + + // Load all configs in before processing them + configs := make([]ConfigFile, 0, len(flFilenames.Value())) + + // We only want to have one global config loaded + globalConfigLoaded := false + for _, flFilename := range flFilenames.Value() { baseDir := filepath.Dir(flFilename) config, err := spec.GitOpsFromFile(flFilename, baseDir, appConfig, logf) @@ -114,17 +123,26 @@ func gitopsCommand() *cli.Command { return err } isGlobalConfig := config.TeamName == nil - if firstFileMustBeGlobal != nil { - switch { - case *firstFileMustBeGlobal && !isGlobalConfig: - return fmt.Errorf("first file %s must be the global config", flFilename) - case !*firstFileMustBeGlobal && isGlobalConfig: - return fmt.Errorf( - "the file %s cannot be the global config, only the first file can be the global config", flFilename, - ) + if isGlobalConfig { + if globalConfigLoaded { + return errors.New("only one global config file may be provided to fleetctl gitops") } - firstFileMustBeGlobal = ptr.Bool(false) + globalConfigLoaded = true } + configFile := ConfigFile{Config: config, Filename: flFilename, IsGlobalConfig: isGlobalConfig} + if isGlobalConfig { + // If it's a global file, put it at the beginning + // of the array so it gets processed first + configs = append([]ConfigFile{configFile}, configs...) + } else { + configs = append(configs, configFile) + } + } + + for _, configFile := range configs { + config := configFile.Config + flFilename := configFile.Filename + isGlobalConfig := configFile.IsGlobalConfig if isGlobalConfig { if noTeamControls.Set() && config.Controls.Set() { diff --git a/cmd/fleetctl/gitops_test.go b/cmd/fleetctl/gitops_test.go index 58ace9dbcc..9eb7096de0 100644 --- a/cmd/fleetctl/gitops_test.go +++ b/cmd/fleetctl/gitops_test.go @@ -1365,13 +1365,17 @@ software: // Files out of order _, err = runAppNoChecks([]string{"gitops", "-f", teamFile.Name(), "-f", globalFile.Name(), "--dry-run"}) - require.Error(t, err) - assert.True(t, strings.Contains(err.Error(), "must be the global config")) + require.NoError(t, err) + + // No global file, only team file + _, err = runAppNoChecks([]string{"gitops", "-f", teamFile.Name(), "--dry-run"}) + require.NoError(t, err) // Global file specified multiple times _, err = runAppNoChecks([]string{"gitops", "-f", globalFile.Name(), "-f", teamFile.Name(), "-f", globalFile.Name(), "--dry-run"}) require.Error(t, err) - assert.True(t, strings.Contains(err.Error(), "only the first file can be the global config")) + fmt.Printf("err.Error(): %v\n", err.Error()) + assert.Contains(t, err.Error(), "only one global config file may be provided") // Duplicate secret _, err = runAppNoChecks([]string{"gitops", "-f", globalFile.Name(), "-f", teamFileDupSecret.Name(), "--dry-run"}) diff --git a/ee/server/service/mdm_test.go b/ee/server/service/mdm_test.go index 5162f20f4d..a1a488500e 100644 --- a/ee/server/service/mdm_test.go +++ b/ee/server/service/mdm_test.go @@ -176,7 +176,7 @@ func TestCountABMTokensAuth(t *testing.T) { {"observer can read", test.UserObserver, false}, {"observer+ can read", test.UserObserverPlus, false}, {"admin can read", test.UserAdmin, false}, - {"tm1 gitops cannot read", test.UserTeamGitOpsTeam1, true}, + {"tm1 gitops can read", test.UserTeamGitOpsTeam1, false}, {"tm1 maintainer can read", test.UserTeamMaintainerTeam1, false}, {"tm1 observer can read", test.UserTeamObserverTeam1, false}, {"tm1 observer+ can read", test.UserTeamObserverPlusTeam1, false}, diff --git a/server/authz/policy.rego b/server/authz/policy.rego index 5e802920cf..f35a83ee9d 100644 --- a/server/authz/policy.rego +++ b/server/authz/policy.rego @@ -61,8 +61,8 @@ allow { # Team admin, maintainer, observer_plus and observer can read global config. allow { object.type == "app_config" - # If role is admin, maintainer, observer_plus or observer on any team. - team_role(subject, subject.teams[_].id) == [admin, maintainer, observer_plus, observer][_] + # If role is admin, gitops, maintainer, observer_plus or observer on any team. + team_role(subject, subject.teams[_].id) == [admin, gitops, maintainer, observer_plus, observer][_] action == read } diff --git a/server/authz/policy_test.go b/server/authz/policy_test.go index 858f322e5e..6f7d600788 100644 --- a/server/authz/policy_test.go +++ b/server/authz/policy_test.go @@ -80,7 +80,7 @@ func TestAuthorizeAppConfig(t *testing.T) { {user: test.UserTeamObserverPlusTeam1, object: config, action: read, allow: true}, {user: test.UserTeamObserverPlusTeam1, object: config, action: write, allow: false}, - {user: test.UserTeamGitOpsTeam1, object: config, action: read, allow: false}, + {user: test.UserTeamGitOpsTeam1, object: config, action: read, allow: true}, {user: test.UserTeamGitOpsTeam1, object: config, action: write, allow: false}, }) } diff --git a/server/service/appconfig_test.go b/server/service/appconfig_test.go index d4ac69804a..6719d90ccf 100644 --- a/server/service/appconfig_test.go +++ b/server/service/appconfig_test.go @@ -133,7 +133,7 @@ func TestAppConfigAuth(t *testing.T) { "team gitops", &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 1}, Role: fleet.RoleGitOps}}}, true, - true, + false, }, { "user without roles", @@ -610,7 +610,7 @@ func TestAppConfigSecretsObfuscated(t *testing.T) { { "team gitops", &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 1}, Role: fleet.RoleGitOps}}}, - true, + false, }, { "user without roles",