From 05eb3385619d294b240dae0f23d79e4b8294074c Mon Sep 17 00:00:00 2001 From: Victor Lyuboslavsky Date: Tue, 11 Jun 2024 13:20:32 -0500 Subject: [PATCH] Enable gitops to create teams with no enroll secrets, or clear enroll secrets for an existing team (#19616) Enable gitops to create teams with no enroll secrets, or clear enroll secrets for an existing team #19332 `fleetctl apply` also gains this extra functionality. In `fleetctl apply` secrets will not be change if one of the following: - secrets is missing from yml - They are blank in yml, like: `secrets:` - They are null in yml, like: `secrets: null` They will only be cleared with `fleetctl apply` if the user explicitly sets them to an empty array, like: - `secrets: []` # Checklist for submitter If some of the following don't apply, delete the relevant line. - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://fleetdm.com/docs/contributing/committing-changes#changes-files) for more information. - [x] Added/updated tests - [x] Manual QA for all new/changed functionality --- changes/ 19332-clear-secrets-with-gitops | 1 + cmd/fleetctl/gitops_test.go | 41 ++++++++++++------- .../testdata/expectedGetTeamsYaml.yml | 1 + .../macosSetupExpectedTeam1And2Empty.yml | 2 + .../macosSetupExpectedTeam1And2Set.yml | 2 + .../testdata/macosSetupExpectedTeam1Empty.yml | 1 + .../testdata/macosSetupExpectedTeam1Set.yml | 1 + ee/server/service/teams.go | 28 ++++++++----- pkg/spec/gitops.go | 3 +- server/fleet/teams.go | 4 +- server/service/integration_core_test.go | 2 +- server/service/integration_mdm_test.go | 4 +- server/service/teams_test.go | 6 +-- 13 files changed, 62 insertions(+), 34 deletions(-) create mode 100644 changes/ 19332-clear-secrets-with-gitops diff --git a/changes/ 19332-clear-secrets-with-gitops b/changes/ 19332-clear-secrets-with-gitops new file mode 100644 index 0000000000..afeb4f987e --- /dev/null +++ b/changes/ 19332-clear-secrets-with-gitops @@ -0,0 +1 @@ +Enable gitops to create teams with no enroll secrets, or clear enroll secrets for an existing team by setting team_settings.secret to nothing or to an empty array. diff --git a/cmd/fleetctl/gitops_test.go b/cmd/fleetctl/gitops_test.go index fb2003c54b..c9e56d636e 100644 --- a/cmd/fleetctl/gitops_test.go +++ b/cmd/fleetctl/gitops_test.go @@ -181,22 +181,29 @@ func TestBasicTeamGitOps(t *testing.T) { CreatedAt: time.Now(), Name: teamName, } + var savedTeam *fleet.Team ds.TeamByNameFunc = func(ctx context.Context, name string) (*fleet.Team, error) { - if name == teamName { - return team, nil + if name == teamName && savedTeam != nil { + return savedTeam, nil } - return nil, nil + return nil, ¬FoundError{} } ds.TeamFunc = func(ctx context.Context, tid uint) (*fleet.Team, error) { if tid == team.ID { - return team, nil + return savedTeam, nil } return nil, nil } + var enrolledTeamSecrets []*fleet.EnrollSecret + ds.NewTeamFunc = func(ctx context.Context, newTeam *fleet.Team) (*fleet.Team, error) { + newTeam.ID = team.ID + savedTeam = newTeam + enrolledTeamSecrets = newTeam.Secrets + return newTeam, nil + } ds.IsEnrollSecretAvailableFunc = func(ctx context.Context, secret string, new bool, teamID *uint) (bool, error) { return true, nil } - var savedTeam *fleet.Team ds.SaveTeamFunc = func(ctx context.Context, team *fleet.Team) (*fleet.Team, error) { savedTeam = team return team, nil @@ -205,10 +212,6 @@ func TestBasicTeamGitOps(t *testing.T) { require.ElementsMatch(t, labels, []string{fleet.BuiltinLabelMacOS14Plus}) return map[string]uint{fleet.BuiltinLabelMacOS14Plus: 1}, nil } - ds.SetOrUpdateMDMAppleDeclarationFunc = func(ctx context.Context, declaration *fleet.MDMAppleDeclaration) (*fleet.MDMAppleDeclaration, error) { - declaration.DeclarationUUID = uuid.NewString() - return declaration, nil - } ds.DeleteMDMAppleDeclarationByNameFunc = func(ctx context.Context, teamID *uint, name string) error { return nil } @@ -216,16 +219,15 @@ func TestBasicTeamGitOps(t *testing.T) { return nil } - var enrolledSecrets []*fleet.EnrollSecret ds.ApplyEnrollSecretsFunc = func(ctx context.Context, teamID *uint, secrets []*fleet.EnrollSecret) error { - enrolledSecrets = secrets + enrolledTeamSecrets = secrets return nil } tmpFile, err := os.CreateTemp(t.TempDir(), "*.yml") require.NoError(t, err) - t.Setenv("TEST_SECRET", secret) + t.Setenv("TEST_SECRET", "") _, err = tmpFile.WriteString( ` @@ -235,7 +237,7 @@ policies: agent_options: name: ${TEST_TEAM_NAME} team_settings: - secrets: [{"secret":"${TEST_SECRET}"}] + secrets: ${TEST_SECRET} `, ) require.NoError(t, err) @@ -255,8 +257,17 @@ team_settings: _ = runAppForTest(t, []string{"gitops", "-f", tmpFile.Name()}) require.NotNil(t, savedTeam) assert.Equal(t, teamName, savedTeam.Name) - require.Len(t, enrolledSecrets, 1) - assert.Equal(t, secret, enrolledSecrets[0].Secret) + assert.Empty(t, enrolledTeamSecrets) + + // The previous run created the team, so let's rerun with an existing team + _ = runAppForTest(t, []string{"gitops", "-f", tmpFile.Name()}) + assert.Empty(t, enrolledTeamSecrets) + + // Add a secret + t.Setenv("TEST_SECRET", fmt.Sprintf("[{\"secret\":\"%s\"}]", secret)) + _ = runAppForTest(t, []string{"gitops", "-f", tmpFile.Name()}) + require.Len(t, enrolledTeamSecrets, 1) + assert.Equal(t, secret, enrolledTeamSecrets[0].Secret) } func TestFullGlobalGitOps(t *testing.T) { diff --git a/cmd/fleetctl/testdata/expectedGetTeamsYaml.yml b/cmd/fleetctl/testdata/expectedGetTeamsYaml.yml index f1315fcf24..f10577a3af 100644 --- a/cmd/fleetctl/testdata/expectedGetTeamsYaml.yml +++ b/cmd/fleetctl/testdata/expectedGetTeamsYaml.yml @@ -29,6 +29,7 @@ spec: enable_release_device_manually: false macos_setup_assistant: scripts: null + secrets: null software: null webhook_settings: host_status_webhook: null diff --git a/cmd/fleetctl/testdata/macosSetupExpectedTeam1And2Empty.yml b/cmd/fleetctl/testdata/macosSetupExpectedTeam1And2Empty.yml index 28f815240d..b5a4c03e5c 100644 --- a/cmd/fleetctl/testdata/macosSetupExpectedTeam1And2Empty.yml +++ b/cmd/fleetctl/testdata/macosSetupExpectedTeam1And2Empty.yml @@ -29,6 +29,7 @@ spec: deadline_days: null grace_period_days: null scripts: null + secrets: null software: null webhook_settings: host_status_webhook: null @@ -63,6 +64,7 @@ spec: deadline_days: null grace_period_days: null scripts: null + secrets: null software: null webhook_settings: host_status_webhook: null diff --git a/cmd/fleetctl/testdata/macosSetupExpectedTeam1And2Set.yml b/cmd/fleetctl/testdata/macosSetupExpectedTeam1And2Set.yml index ef911ec34f..a0d15fddd7 100644 --- a/cmd/fleetctl/testdata/macosSetupExpectedTeam1And2Set.yml +++ b/cmd/fleetctl/testdata/macosSetupExpectedTeam1And2Set.yml @@ -29,6 +29,7 @@ spec: deadline_days: null grace_period_days: null scripts: null + secrets: null software: null webhook_settings: host_status_webhook: null @@ -63,6 +64,7 @@ spec: deadline_days: null grace_period_days: null scripts: null + secrets: null software: null webhook_settings: host_status_webhook: null diff --git a/cmd/fleetctl/testdata/macosSetupExpectedTeam1Empty.yml b/cmd/fleetctl/testdata/macosSetupExpectedTeam1Empty.yml index 19f92edbc0..8a6762468c 100644 --- a/cmd/fleetctl/testdata/macosSetupExpectedTeam1Empty.yml +++ b/cmd/fleetctl/testdata/macosSetupExpectedTeam1Empty.yml @@ -29,6 +29,7 @@ spec: windows_settings: custom_settings: null scripts: null + secrets: null software: null webhook_settings: host_status_webhook: null diff --git a/cmd/fleetctl/testdata/macosSetupExpectedTeam1Set.yml b/cmd/fleetctl/testdata/macosSetupExpectedTeam1Set.yml index 9862a2d66d..2aac4b1481 100644 --- a/cmd/fleetctl/testdata/macosSetupExpectedTeam1Set.yml +++ b/cmd/fleetctl/testdata/macosSetupExpectedTeam1Set.yml @@ -28,6 +28,7 @@ spec: deadline_days: null grace_period_days: null scripts: null + secrets: null software: null webhook_settings: host_status_webhook: null diff --git a/ee/server/service/teams.go b/ee/server/service/teams.go index 452bf80f9b..2b8852ff2a 100644 --- a/ee/server/service/teams.go +++ b/ee/server/service/teams.go @@ -773,10 +773,17 @@ func (svc *Service) ApplyTeamSpecs(ctx context.Context, specs []*fleet.TeamSpec, for _, spec := range specs { var secrets []*fleet.EnrollSecret - for _, secret := range spec.Secrets { - secrets = append(secrets, &fleet.EnrollSecret{ - Secret: secret.Secret, - }) + // When secrets slice is empty, all secrets are removed. + // When secrets slice is nil, existing secrets are kept. + if spec.Secrets != nil { + secrets = make([]*fleet.EnrollSecret, 0, len(*spec.Secrets)) + for _, secret := range *spec.Secrets { + secrets = append( + secrets, &fleet.EnrollSecret{ + Secret: secret.Secret, + }, + ) + } } var create bool @@ -804,7 +811,7 @@ func (svc *Service) ApplyTeamSpecs(ctx context.Context, specs []*fleet.TeamSpec, } } } - if len(spec.Secrets) > fleet.MaxEnrollSecretsCount { + if len(secrets) > fleet.MaxEnrollSecretsCount { return nil, ctxerr.Wrap(ctx, fleet.NewInvalidArgumentError("secrets", "too many secrets"), "validate secrets") } if err := spec.MDM.MacOSUpdates.Validate(); err != nil { @@ -816,8 +823,9 @@ func (svc *Service) ApplyTeamSpecs(ctx context.Context, specs []*fleet.TeamSpec, if create { - // create a new team enroll secret if none is provided for a new team. - if len(secrets) == 0 { + // create a new team enroll secret if none is provided for a new team, + // unless the user explicitly passed in an empty array + if secrets == nil { secret, err := server.GenerateRandomText(fleet.EnrollSecretDefaultLength) if err != nil { return nil, ctxerr.Wrap(ctx, err, "generate enroll secret string") @@ -1125,7 +1133,7 @@ func (svc *Service) editTeamFromSpec( team.Config.Software = spec.Software } - if len(secrets) > 0 { + if secrets != nil { team.Secrets = secrets } @@ -1179,8 +1187,8 @@ func (svc *Service) editTeamFromSpec( return err } - // only replace enroll secrets if at least one is provided (#6774) - if len(secrets) > 0 { + // If no secrets are provided and user did not explicitly specify an empty list, do not replace secrets. (#6774) + if secrets != nil { if err := svc.ds.ApplyEnrollSecrets(ctx, ptr.Uint(team.ID), secrets); err != nil { return err } diff --git a/pkg/spec/gitops.go b/pkg/spec/gitops.go index 0fbebe70b5..a416c33787 100644 --- a/pkg/spec/gitops.go +++ b/pkg/spec/gitops.go @@ -244,7 +244,8 @@ func parseSecrets(result *GitOps, multiError *multierror.Error) *multierror.Erro return multierror.Append(multiError, errors.New("'team_settings.secrets' is required")) } } - var enrollSecrets []*fleet.EnrollSecret + // When secrets slice is empty, all secrets are removed. + enrollSecrets := make([]*fleet.EnrollSecret, 0) if rawSecrets != nil { secrets, ok := rawSecrets.([]interface{}) if !ok { diff --git a/server/fleet/teams.go b/server/fleet/teams.go index 984d295343..ebdf8a7eb1 100644 --- a/server/fleet/teams.go +++ b/server/fleet/teams.go @@ -419,7 +419,7 @@ type TeamSpec struct { // set to the agent options JSON object. AgentOptions json.RawMessage `json:"agent_options,omitempty"` // marshals as "null" if omitempty is not set HostExpirySettings *HostExpirySettings `json:"host_expiry_settings,omitempty"` - Secrets []EnrollSecret `json:"secrets,omitempty"` + Secrets *[]EnrollSecret `json:"secrets,omitempty"` Features *json.RawMessage `json:"features"` MDM TeamSpecMDM `json:"mdm"` Scripts optjson.Slice[string] `json:"scripts"` @@ -486,7 +486,7 @@ func TeamSpecFromTeam(t *Team) (*TeamSpec, error) { Name: t.Name, AgentOptions: agentOptions, Features: &featuresJSON, - Secrets: secrets, + Secrets: &secrets, MDM: mdmSpec, HostExpirySettings: &t.Config.HostExpirySettings, WebhookSettings: webhookSettings, diff --git a/server/service/integration_core_test.go b/server/service/integration_core_test.go index 802705617e..c3d0d8ab77 100644 --- a/server/service/integration_core_test.go +++ b/server/service/integration_core_test.go @@ -6012,7 +6012,7 @@ func (s *integrationTestSuite) TestPremiumEndpointsWithoutLicense() { // apply team specs var specResp applyTeamSpecsResponse - teamSpecs := applyTeamSpecsRequest{Specs: []*fleet.TeamSpec{{Name: "newteam", Secrets: []fleet.EnrollSecret{{Secret: "ABC"}}}}} + teamSpecs := applyTeamSpecsRequest{Specs: []*fleet.TeamSpec{{Name: "newteam", Secrets: &[]fleet.EnrollSecret{{Secret: "ABC"}}}}} s.DoJSON("POST", "/api/latest/fleet/spec/teams", teamSpecs, http.StatusPaymentRequired, &specResp) // modify team agent options diff --git a/server/service/integration_mdm_test.go b/server/service/integration_mdm_test.go index 4f95a5395d..e61a5e2ad7 100644 --- a/server/service/integration_mdm_test.go +++ b/server/service/integration_mdm_test.go @@ -71,7 +71,7 @@ import ( func TestIntegrationsMDM(t *testing.T) { testingSuite := new(integrationMDMTestSuite) - testingSuite.s = &testingSuite.Suite + testingSuite.withServer.s = &testingSuite.Suite suite.Run(t, testingSuite) } @@ -2316,7 +2316,7 @@ func (s *integrationMDMTestSuite) TestFleetdConfiguration() { // create an enroll secret for the team teamSpecs := applyTeamSpecsRequest{Specs: []*fleet.TeamSpec{{ Name: tm.Name, - Secrets: []fleet.EnrollSecret{{Secret: t.Name() + "team-secret"}}, + Secrets: &[]fleet.EnrollSecret{{Secret: t.Name() + "team-secret"}}, }}} s.Do("POST", "/api/latest/fleet/spec/teams", teamSpecs, http.StatusOK) diff --git a/server/service/teams_test.go b/server/service/teams_test.go index c145c34714..9011e8bce7 100644 --- a/server/service/teams_test.go +++ b/server/service/teams_test.go @@ -431,7 +431,7 @@ func TestApplyTeamSpecEnrollSecretForNewTeams(t *testing.T) { return false, nil } _, err := svc.ApplyTeamSpecs( - ctx, []*fleet.TeamSpec{{Name: "Foo", Secrets: []fleet.EnrollSecret{enrollSecret}}}, + ctx, []*fleet.TeamSpec{{Name: "Foo", Secrets: &[]fleet.EnrollSecret{enrollSecret}}}, fleet.ApplyTeamSpecOptions{ApplySpecOptions: fleet.ApplySpecOptions{DryRun: true}}, ) assert.ErrorContains(t, err, "is already being used") @@ -441,14 +441,14 @@ func TestApplyTeamSpecEnrollSecretForNewTeams(t *testing.T) { return true, nil } _, err = svc.ApplyTeamSpecs( - ctx, []*fleet.TeamSpec{{Name: "Foo", Secrets: []fleet.EnrollSecret{enrollSecret}}}, + ctx, []*fleet.TeamSpec{{Name: "Foo", Secrets: &[]fleet.EnrollSecret{enrollSecret}}}, fleet.ApplyTeamSpecOptions{ApplySpecOptions: fleet.ApplySpecOptions{DryRun: true}}, ) assert.NoError(t, err) assert.False(t, ds.NewTeamFuncInvoked) _, err = svc.ApplyTeamSpecs( - ctx, []*fleet.TeamSpec{{Name: "Foo", Secrets: []fleet.EnrollSecret{enrollSecret}}}, fleet.ApplyTeamSpecOptions{}, + ctx, []*fleet.TeamSpec{{Name: "Foo", Secrets: &[]fleet.EnrollSecret{enrollSecret}}}, fleet.ApplyTeamSpecOptions{}, ) require.NoError(t, err) require.True(t, ds.TeamByNameFuncInvoked)