Validate team and appconfig payloads, with dry-run and force modes (#7731)

ee/server/service/teams.go

@@ -13,6 +13,7 @@ import (
 	"github.com/fleetdm/fleet/v4/server/contexts/viewer"
 	"github.com/fleetdm/fleet/v4/server/fleet"
 	"github.com/fleetdm/fleet/v4/server/ptr"
+	"github.com/go-kit/kit/log/level"
 )
 
 func (svc *Service) NewTeam(ctx context.Context, p fleet.TeamPayload) (*fleet.Team, error) {
@@ -132,7 +133,7 @@ func (svc *Service) ModifyTeam(ctx context.Context, teamID uint, payload fleet.T
 	return svc.ds.SaveTeam(ctx, team)
 }
 
-func (svc *Service) ModifyTeamAgentOptions(ctx context.Context, teamID uint, options json.RawMessage) (*fleet.Team, error) {
+func (svc *Service) ModifyTeamAgentOptions(ctx context.Context, teamID uint, teamOptions json.RawMessage, applyOptions fleet.ApplySpecOptions) (*fleet.Team, error) {
 	if err := svc.authz.Authorize(ctx, &fleet.Team{ID: teamID}, fleet.ActionWrite); err != nil {
 		return nil, err
 	}
@@ -142,12 +143,23 @@ func (svc *Service) ModifyTeamAgentOptions(ctx context.Context, teamID uint, opt
 		return nil, err
 	}
 
-	if options != nil {
-		team.Config.AgentOptions = &options
-		// TODO(mna): validate agent options before saving
+	if teamOptions != nil {
+		if err := fleet.ValidateJSONAgentOptions(teamOptions); err != nil {
+			if applyOptions.Force && !applyOptions.DryRun {
+				level.Info(svc.logger).Log("err", err, "msg", "force-apply team agent options with validation errors")
+			}
+			if !applyOptions.Force {
+				return nil, ctxerr.Wrap(ctx, &fleet.BadRequestError{Message: err.Error()}, "validate agent options")
+			}
+		}
+	}
+	if applyOptions.DryRun {
+		return team, nil
+	}
+
+	if teamOptions != nil {
+		team.Config.AgentOptions = &teamOptions
 	} else {
-		// TODO(mna): in NewTeam, we set AgentOptions to the global config, why
-		// do we allow setting it to nil here?
 		team.Config.AgentOptions = nil
 	}
 
@@ -356,7 +368,7 @@ func (svc *Service) ModifyTeamEnrollSecrets(ctx context.Context, teamID uint, se
 	return newSecrets, nil
 }
 
-func (svc Service) ApplyTeamSpecs(ctx context.Context, specs []*fleet.TeamSpec) error {
+func (svc *Service) ApplyTeamSpecs(ctx context.Context, specs []*fleet.TeamSpec, applyOpts fleet.ApplySpecOptions) error {
 	if err := svc.authz.Authorize(ctx, &fleet.Team{}, fleet.ActionRead); err != nil {
 		return err
 	}
@@ -401,11 +413,36 @@ func (svc Service) ApplyTeamSpecs(ctx context.Context, specs []*fleet.TeamSpec)
 			})
 		}
 
+		var create bool
 		team, err := svc.ds.TeamByName(ctx, spec.Name)
 		switch {
 		case err == nil:
 			// OK
 		case ctxerr.Cause(err) == sql.ErrNoRows:
+			if spec.Name == "" {
+				return fleet.NewInvalidArgumentError("name", "name may not be empty")
+			}
+			create = true
+		default:
+			return err
+		}
+
+		if spec.AgentOptions != nil {
+			if err := fleet.ValidateJSONAgentOptions(*spec.AgentOptions); err != nil {
+				if applyOpts.Force && !applyOpts.DryRun {
+					level.Info(svc.logger).Log("err", err, "msg", "force-apply team agent options with validation errors")
+				}
+				if !applyOpts.Force {
+					return ctxerr.Wrap(ctx, &fleet.BadRequestError{Message: err.Error()}, "validate agent options")
+				}
+			}
+		}
+
+		if applyOpts.DryRun {
+			continue
+		}
+
+		if create {
 			team, err := svc.createTeamFromSpec(ctx, spec, appConfig, secrets)
 			if err != nil {
 				return ctxerr.Wrap(ctx, err, "creating team from spec")
@@ -415,8 +452,6 @@ func (svc Service) ApplyTeamSpecs(ctx context.Context, specs []*fleet.TeamSpec)
 				Name: team.Name,
 			})
 			continue
-		default:
-			return err
 		}
 
 		if err := svc.editTeamFromSpec(ctx, team, spec, secrets); err != nil {
@@ -429,19 +464,20 @@ func (svc Service) ApplyTeamSpecs(ctx context.Context, specs []*fleet.TeamSpec)
 		})
 	}
 
-	if err := svc.ds.NewActivity(
-		ctx,
-		authz.UserFromContext(ctx),
-		fleet.ActivityTypeAppliedSpecTeam,
-		&map[string]interface{}{"teams": details},
-	); err != nil {
-		return ctxerr.Wrap(ctx, err, "create applied team spec activity")
+	if len(details) > 0 {
+		if err := svc.ds.NewActivity(
+			ctx,
+			authz.UserFromContext(ctx),
+			fleet.ActivityTypeAppliedSpecTeam,
+			&map[string]interface{}{"teams": details},
+		); err != nil {
+			return ctxerr.Wrap(ctx, err, "create applied team spec activity")
+		}
 	}
 	return nil
 }
 
 func (svc Service) createTeamFromSpec(ctx context.Context, spec *fleet.TeamSpec, defaults *fleet.AppConfig, secrets []*fleet.EnrollSecret) (*fleet.Team, error) {
-	// TODO(mna): validate agent options before saving
 	agentOptions := spec.AgentOptions
 	if agentOptions == nil {
 		agentOptions = defaults.AgentOptions
@@ -470,7 +506,6 @@ func (svc Service) createTeamFromSpec(ctx context.Context, spec *fleet.TeamSpec,
 
 func (svc Service) editTeamFromSpec(ctx context.Context, team *fleet.Team, spec *fleet.TeamSpec, secrets []*fleet.EnrollSecret) error {
 	team.Name = spec.Name
-	// TODO(mna): validate agent options before saving, and we allow nil here instead of defaulting to global?
 	team.Config.AgentOptions = spec.AgentOptions
 
 	// replace (don't merge) the features with the new ones, using a config