From 267b2f133b65860c27fbdfa81c8ba74c4b6d2843 Mon Sep 17 00:00:00 2001 From: Xing Yahao Date: Thu, 2 May 2024 01:06:35 +0900 Subject: [PATCH] feat: make api plan apply support workflow hooks (#4482) Co-authored-by: PePe Amengual --- server/controllers/api_controller.go | 64 ++++++++++++++----- server/controllers/api_controller_test.go | 30 ++++++--- server/events/command/context.go | 3 + server/events/models/models.go | 2 + .../post_workflow_hooks_command_runner.go | 3 +- .../pre_workflow_hooks_command_runner.go | 13 +++- server/server.go | 23 ++++--- 7 files changed, 97 insertions(+), 41 deletions(-) diff --git a/server/controllers/api_controller.go b/server/controllers/api_controller.go index 43e316bbdf..c48c99b41d 100644 --- a/server/controllers/api_controller.go +++ b/server/controllers/api_controller.go @@ -20,16 +20,19 @@ import ( const atlantisTokenHeader = "X-Atlantis-Token" type APIController struct { - APISecret []byte - Locker locking.Locker - Logger logging.SimpleLogging - Parser events.EventParsing - ProjectCommandBuilder events.ProjectCommandBuilder - ProjectPlanCommandRunner events.ProjectPlanCommandRunner - ProjectApplyCommandRunner events.ProjectApplyCommandRunner - RepoAllowlistChecker *events.RepoAllowlistChecker - Scope tally.Scope - VCSClient vcs.Client + APISecret []byte + Locker locking.Locker + Logger logging.SimpleLogging + Parser events.EventParsing + ProjectCommandBuilder events.ProjectCommandBuilder + ProjectPlanCommandRunner events.ProjectPlanCommandRunner + ProjectApplyCommandRunner events.ProjectApplyCommandRunner + FailOnPreWorkflowHookError bool + PreWorkflowHooksCommandRunner events.PreWorkflowHooksCommandRunner + PostWorkflowHooksCommandRunner events.PostWorkflowHooksCommandRunner + RepoAllowlistChecker *events.RepoAllowlistChecker + Scope tally.Scope + VCSClient vcs.Client } type APIRequest struct { @@ -44,7 +47,7 @@ type APIRequest struct { } } -func (a *APIRequest) getCommands(ctx *command.Context, cmdBuilder func(*command.Context, *events.CommentCommand) ([]command.ProjectContext, error)) ([]command.ProjectContext, error) { +func (a *APIRequest) getCommands(ctx *command.Context, cmdBuilder func(*command.Context, *events.CommentCommand) ([]command.ProjectContext, error)) ([]command.ProjectContext, []*events.CommentCommand, error) { cc := make([]*events.CommentCommand, 0) for _, project := range a.Projects { @@ -63,12 +66,12 @@ func (a *APIRequest) getCommands(ctx *command.Context, cmdBuilder func(*command. for _, commentCommand := range cc { projectCmds, err := cmdBuilder(ctx, commentCommand) if err != nil { - return nil, fmt.Errorf("failed to build command: %v", err) + return nil, nil, fmt.Errorf("failed to build command: %v", err) } cmds = append(cmds, projectCmds...) } - return cmds, nil + return cmds, cc, nil } func (a *APIController) apiReportError(w http.ResponseWriter, code int, err error) { @@ -142,29 +145,55 @@ func (a *APIController) Apply(w http.ResponseWriter, r *http.Request) { } func (a *APIController) apiPlan(request *APIRequest, ctx *command.Context) (*command.Result, error) { - cmds, err := request.getCommands(ctx, a.ProjectCommandBuilder.BuildPlanCommands) + cmds, cc, err := request.getCommands(ctx, a.ProjectCommandBuilder.BuildPlanCommands) if err != nil { return nil, err } var projectResults []command.ProjectResult - for _, cmd := range cmds { + for i, cmd := range cmds { + err = a.PreWorkflowHooksCommandRunner.RunPreHooks(ctx, cc[i]) + if err != nil { + ctx.Log.Err("Error running pre-workflow hooks %s.", err) + if a.FailOnPreWorkflowHookError { + return nil, err + } + } + res := a.ProjectPlanCommandRunner.Plan(cmd) projectResults = append(projectResults, res) + + err = a.PostWorkflowHooksCommandRunner.RunPostHooks(ctx, cc[i]) + if err != nil { + ctx.Log.Err("Error running post-workflow hooks %s.", err) + } } return &command.Result{ProjectResults: projectResults}, nil } func (a *APIController) apiApply(request *APIRequest, ctx *command.Context) (*command.Result, error) { - cmds, err := request.getCommands(ctx, a.ProjectCommandBuilder.BuildApplyCommands) + cmds, cc, err := request.getCommands(ctx, a.ProjectCommandBuilder.BuildApplyCommands) if err != nil { return nil, err } var projectResults []command.ProjectResult - for _, cmd := range cmds { + for i, cmd := range cmds { + err = a.PreWorkflowHooksCommandRunner.RunPreHooks(ctx, cc[i]) + if err != nil { + ctx.Log.Err("Error running pre-workflow hooks %s.", err) + if a.FailOnPreWorkflowHookError { + return nil, err + } + } + res := a.ProjectApplyCommandRunner.Apply(cmd) projectResults = append(projectResults, res) + + err = a.PostWorkflowHooksCommandRunner.RunPostHooks(ctx, cc[i]) + if err != nil { + ctx.Log.Err("Error running post-workflow hooks %s.", err) + } } return &command.Result{ProjectResults: projectResults}, nil } @@ -223,6 +252,7 @@ func (a *APIController) apiParseAndValidate(r *http.Request) (*APIRequest, *comm }, Scope: a.Scope, Log: a.Logger, + API: true, }, http.StatusOK, nil } diff --git a/server/controllers/api_controller_test.go b/server/controllers/api_controller_test.go index 1f2370ef08..3b3aa520aa 100644 --- a/server/controllers/api_controller_test.go +++ b/server/controllers/api_controller_test.go @@ -86,17 +86,27 @@ func setup(t *testing.T) (controllers.APIController, *MockProjectCommandBuilder, ApplySuccess: "success", }) + preWorkflowHooksCommandRunner := NewMockPreWorkflowHooksCommandRunner() + + When(preWorkflowHooksCommandRunner.RunPreHooks(Any[*command.Context](), Any[*events.CommentCommand]())).ThenReturn(nil) + + postWorkflowHooksCommandRunner := NewMockPostWorkflowHooksCommandRunner() + + When(postWorkflowHooksCommandRunner.RunPostHooks(Any[*command.Context](), Any[*events.CommentCommand]())).ThenReturn(nil) + ac := controllers.APIController{ - APISecret: []byte(atlantisToken), - Locker: locker, - Logger: logger, - Scope: scope, - Parser: parser, - ProjectCommandBuilder: projectCommandBuilder, - ProjectPlanCommandRunner: projectCommandRunner, - ProjectApplyCommandRunner: projectCommandRunner, - VCSClient: vcsClient, - RepoAllowlistChecker: repoAllowlistChecker, + APISecret: []byte(atlantisToken), + Locker: locker, + Logger: logger, + Scope: scope, + Parser: parser, + ProjectCommandBuilder: projectCommandBuilder, + ProjectPlanCommandRunner: projectCommandRunner, + ProjectApplyCommandRunner: projectCommandRunner, + PreWorkflowHooksCommandRunner: preWorkflowHooksCommandRunner, + PostWorkflowHooksCommandRunner: postWorkflowHooksCommandRunner, + VCSClient: vcsClient, + RepoAllowlistChecker: repoAllowlistChecker, } return ac, projectCommandBuilder, projectCommandRunner } diff --git a/server/events/command/context.go b/server/events/command/context.go index 1d6748915c..623c49588a 100644 --- a/server/events/command/context.go +++ b/server/events/command/context.go @@ -43,4 +43,7 @@ type Context struct { ClearPolicyApproval bool Trigger Trigger + + // API is true if plan/apply by API endpoints + API bool } diff --git a/server/events/models/models.go b/server/events/models/models.go index 4ff5bc339d..a23410a69b 100644 --- a/server/events/models/models.go +++ b/server/events/models/models.go @@ -653,6 +653,8 @@ type WorkflowHookCommandContext struct { // Workspace is the Terraform workspace this project is in. It will always // be set. Workspace string + // API is true if plan/apply by API endpoints + API bool } // PlanSuccessStats holds stats for a plan. diff --git a/server/events/post_workflow_hooks_command_runner.go b/server/events/post_workflow_hooks_command_runner.go index e1f7f10a9d..f5fe0c5245 100644 --- a/server/events/post_workflow_hooks_command_runner.go +++ b/server/events/post_workflow_hooks_command_runner.go @@ -79,6 +79,7 @@ func (w *DefaultPostWorkflowHooksCommandRunner) RunPostHooks(ctx *command.Contex Verbose: false, EscapedCommentArgs: escapedArgs, CommandName: cmd.Name.String(), + API: ctx.API, }, postWorkflowHooks, repoDir) @@ -124,7 +125,7 @@ func (w *DefaultPostWorkflowHooksCommandRunner) runHooks( shellArgs = "-c" } url, err := w.Router.GenerateProjectWorkflowHookURL(ctx.HookID) - if err != nil { + if err != nil && !ctx.API { return err } diff --git a/server/events/pre_workflow_hooks_command_runner.go b/server/events/pre_workflow_hooks_command_runner.go index 0e81f15ab9..70462765a3 100644 --- a/server/events/pre_workflow_hooks_command_runner.go +++ b/server/events/pre_workflow_hooks_command_runner.go @@ -91,6 +91,7 @@ func (w *DefaultPreWorkflowHooksCommandRunner) RunPreHooks(ctx *command.Context, Verbose: false, EscapedCommentArgs: escapedArgs, CommandName: cmd.Name.String(), + API: ctx.API, }, preWorkflowHooks, repoDir) @@ -135,13 +136,17 @@ func (w *DefaultPreWorkflowHooksCommandRunner) runHooks( shellArgs = "-c" } url, err := w.Router.GenerateProjectWorkflowHookURL(ctx.HookID) - if err != nil { + if err != nil && !ctx.API { return err } if err := w.CommitStatusUpdater.UpdatePreWorkflowHook(ctx.Log, ctx.Pull, models.PendingCommitStatus, ctx.HookDescription, "", url); err != nil { ctx.Log.Warn("unable to update pre workflow hook status: %s", err) - return err + ctx.Log.Info("is api? %v", ctx.API) + if !ctx.API { + ctx.Log.Info("is api? %v", ctx.API) + return err + } } _, runtimeDesc, err := w.PreWorkflowHookRunner.Run(ctx, hook.RunCommand, shell, shellArgs, repoDir) @@ -155,7 +160,9 @@ func (w *DefaultPreWorkflowHooksCommandRunner) runHooks( if err := w.CommitStatusUpdater.UpdatePreWorkflowHook(ctx.Log, ctx.Pull, models.SuccessCommitStatus, ctx.HookDescription, runtimeDesc, url); err != nil { ctx.Log.Warn("unable to update pre workflow hook status: %s", err) - return err + if !ctx.API { + return err + } } } diff --git a/server/server.go b/server/server.go index c1a1ff50df..215b5dd02b 100644 --- a/server/server.go +++ b/server/server.go @@ -876,16 +876,19 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { StatsScope: statsScope.SubScope("api"), } apiController := &controllers.APIController{ - APISecret: []byte(userConfig.APISecret), - Locker: lockingClient, - Logger: logger, - Parser: eventParser, - ProjectCommandBuilder: projectCommandBuilder, - ProjectPlanCommandRunner: instrumentedProjectCmdRunner, - ProjectApplyCommandRunner: instrumentedProjectCmdRunner, - RepoAllowlistChecker: repoAllowlist, - Scope: statsScope.SubScope("api"), - VCSClient: vcsClient, + APISecret: []byte(userConfig.APISecret), + Locker: lockingClient, + Logger: logger, + Parser: eventParser, + ProjectCommandBuilder: projectCommandBuilder, + ProjectPlanCommandRunner: instrumentedProjectCmdRunner, + ProjectApplyCommandRunner: instrumentedProjectCmdRunner, + FailOnPreWorkflowHookError: userConfig.FailOnPreWorkflowHookError, + PreWorkflowHooksCommandRunner: preWorkflowHooksCommandRunner, + PostWorkflowHooksCommandRunner: postWorkflowHooksCommandRunner, + RepoAllowlistChecker: repoAllowlist, + Scope: statsScope.SubScope("api"), + VCSClient: vcsClient, } eventsController := &events_controllers.VCSEventsController{