feat!: step status reporting + fix pull request context overwrite bug (…

…#1090)

* init commit

* add validation

* bring in types and add some tests

* fix hook skip and add some tests

* address feedback

* address feedback

api/build/compile_publish.go

@@ -4,6 +4,7 @@ package build
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/http"
 	"strconv"
@@ -45,7 +46,7 @@ func CompileAndPublish(
 	scm scm.Service,
 	compiler compiler.Engine,
 	queue queue.Service,
-) (bool, *pipeline.Build, *types.Item, error) {
+) (*pipeline.Build, *types.Item, error) {
 	logrus.Debugf("generating queue items for build %s/%d", cfg.Repo.GetFullName(), cfg.Build.GetNumber())
 
 	// assign variables from form for readibility
@@ -61,7 +62,7 @@ func CompileAndPublish(
 		retErr := fmt.Errorf("%s: failed to get owner for %s: %w", baseErr, r.GetFullName(), err)
 		util.HandleError(c, http.StatusBadRequest, retErr)
 
-		return false, nil, nil, retErr
+		return nil, nil, retErr
 	}
 
 	// confirm current repo owner has at least write access to repo (needed for status update later)
@@ -70,7 +71,7 @@ func CompileAndPublish(
 		retErr := fmt.Errorf("unable to publish build to queue: repository owner %s no longer has write access to repository %s", u.GetName(), r.GetFullName())
 		util.HandleError(c, http.StatusUnauthorized, retErr)
 
-		return false, nil, nil, retErr
+		return nil, nil, retErr
 	}
 
 	// get pull request number from build if event is pull_request or issue_comment
@@ -81,7 +82,7 @@ func CompileAndPublish(
 			retErr := fmt.Errorf("%s: failed to get pull request number for %s: %w", baseErr, r.GetFullName(), err)
 			util.HandleError(c, http.StatusBadRequest, retErr)
 
-			return false, nil, nil, retErr
+			return nil, nil, retErr
 		}
 	}
 
@@ -92,15 +93,15 @@ func CompileAndPublish(
 			retErr := fmt.Errorf("%s: failed to get pull request number for %s: %w", baseErr, r.GetFullName(), err)
 			util.HandleError(c, http.StatusBadRequest, retErr)
 
-			return false, nil, nil, retErr
+			return nil, nil, retErr
 		}
 
 		commit, branch, baseref, headref, err := scm.GetPullRequest(c, u, r, prNum)
 		if err != nil {
 			retErr := fmt.Errorf("%s: failed to get pull request info for %s: %w", baseErr, r.GetFullName(), err)
 			util.HandleError(c, http.StatusInternalServerError, retErr)
 
-			return false, nil, nil, retErr
+			return nil, nil, retErr
 		}
 
 		b.SetCommit(commit)
@@ -117,7 +118,7 @@ func CompileAndPublish(
 			retErr := fmt.Errorf("failed to get commit for repo %s on %s branch: %w", r.GetFullName(), r.GetBranch(), err)
 			util.HandleError(c, http.StatusInternalServerError, retErr)
 
-			return false, nil, nil, retErr
+			return nil, nil, retErr
 		}
 
 		b.SetCommit(commit)
@@ -134,7 +135,7 @@ func CompileAndPublish(
 		retErr := fmt.Errorf("%s: unable to get count of builds for repo %s", baseErr, r.GetFullName())
 		util.HandleError(c, http.StatusBadRequest, retErr)
 
-		return false, nil, nil, retErr
+		return nil, nil, retErr
 	}
 
 	logrus.Debugf("currently %d builds running on repo %s", builds, r.GetFullName())
@@ -144,7 +145,7 @@ func CompileAndPublish(
 		retErr := fmt.Errorf("%s: repo %s has exceeded the concurrent build limit of %d", baseErr, r.GetFullName(), r.GetBuildLimit())
 		util.HandleError(c, http.StatusBadRequest, retErr)
 
-		return false, nil, nil, retErr
+		return nil, nil, retErr
 	}
 
 	// update fields in build object
@@ -173,7 +174,7 @@ func CompileAndPublish(
 			retErr := fmt.Errorf("%s: failed to get changeset for %s: %w", baseErr, r.GetFullName(), err)
 			util.HandleError(c, http.StatusInternalServerError, retErr)
 
-			return false, nil, nil, retErr
+			return nil, nil, retErr
 		}
 	}
 
@@ -185,7 +186,7 @@ func CompileAndPublish(
 			retErr := fmt.Errorf("%s: failed to get changeset for %s: %w", baseErr, r.GetFullName(), err)
 			util.HandleError(c, http.StatusInternalServerError, retErr)
 
-			return false, nil, nil, retErr
+			return nil, nil, retErr
 		}
 	}
 
@@ -225,7 +226,7 @@ func CompileAndPublish(
 
 				util.HandleError(c, http.StatusNotFound, retErr)
 
-				return false, nil, nil, retErr
+				return nil, nil, retErr
 			}
 		} else {
 			pipelineFile = pipeline.GetData()
@@ -246,7 +247,7 @@ func CompileAndPublish(
 
 			util.HandleError(c, http.StatusBadRequest, retErr)
 
-			return false, nil, nil, retErr
+			return nil, nil, retErr
 		}
 
 		// update DB record of repo (repo) with any changes captured from webhook payload (r)
@@ -297,7 +298,7 @@ func CompileAndPublish(
 			retErr := fmt.Errorf("%s: %w", baseErr, err)
 			util.HandleError(c, http.StatusInternalServerError, retErr)
 
-			return false, nil, nil, retErr
+			return nil, nil, retErr
 		}
 
 		// reset the pipeline type for the repo
@@ -320,12 +321,11 @@ func CompileAndPublish(
 				logrus.Errorf("unable to set commit status for %s/%d: %v", repo.GetFullName(), b.GetNumber(), err)
 			}
 
-			return false,
-				nil,
+			return nil,
 				&types.Item{
 					Build: b,
 				},
-				nil
+				errors.New(skip)
 		}
 
 		// check if the pipeline did not already exist in the database
@@ -350,7 +350,7 @@ func CompileAndPublish(
 
 				util.HandleError(c, http.StatusBadRequest, retErr)
 
-				return false, nil, nil, retErr
+				return nil, nil, retErr
 			}
 		}
 
@@ -363,7 +363,7 @@ func CompileAndPublish(
 		//   using the same Number and thus create a constraint
 		//   conflict; consider deleting the partially created
 		//   build object in the database
-		err = PlanBuild(c, database, p, b, repo)
+		err = PlanBuild(c, database, scm, p, b, repo)
 		if err != nil {
 			retErr := fmt.Errorf("%s: %w", baseErr, err)
 
@@ -382,7 +382,7 @@ func CompileAndPublish(
 
 			util.HandleError(c, http.StatusInternalServerError, retErr)
 
-			return false, nil, nil, retErr
+			return nil, nil, retErr
 		}
 
 		// break the loop because everything was successful
@@ -395,23 +395,23 @@ func CompileAndPublish(
 		retErr := fmt.Errorf("%s: failed to update repo %s: %w", baseErr, repo.GetFullName(), err)
 		util.HandleError(c, http.StatusBadRequest, retErr)
 
-		return false, nil, nil, retErr
+		return nil, nil, retErr
 	}
 
 	// return error if pipeline didn't get populated
 	if p == nil {
 		retErr := fmt.Errorf("%s: failed to set pipeline for %s: %w", baseErr, repo.GetFullName(), err)
 		util.HandleError(c, http.StatusBadRequest, retErr)
 
-		return false, nil, nil, retErr
+		return nil, nil, retErr
 	}
 
 	// return error if build didn't get populated
 	if b == nil {
 		retErr := fmt.Errorf("%s: failed to set build for %s: %w", baseErr, repo.GetFullName(), err)
 		util.HandleError(c, http.StatusBadRequest, retErr)
 
-		return false, nil, nil, retErr
+		return nil, nil, retErr
 	}
 
 	// send API call to capture the triggered build
@@ -420,7 +420,7 @@ func CompileAndPublish(
 		retErr := fmt.Errorf("%s: failed to get new build %s/%d: %w", baseErr, repo.GetFullName(), b.GetNumber(), err)
 		util.HandleError(c, http.StatusInternalServerError, retErr)
 
-		return false, nil, nil, retErr
+		return nil, nil, retErr
 	}
 
 	// determine queue route
@@ -431,7 +431,7 @@ func CompileAndPublish(
 		// error out the build
 		CleanBuild(c, database, b, nil, nil, retErr)
 
-		return false, nil, nil, retErr
+		return nil, nil, retErr
 	}
 
 	// temporarily set host to the route before it gets picked up by a worker
@@ -443,10 +443,10 @@ func CompileAndPublish(
 		retErr := fmt.Errorf("unable to publish build executable for %s/%d: %w", repo.GetFullName(), b.GetNumber(), err)
 		util.HandleError(c, http.StatusInternalServerError, retErr)
 
-		return false, nil, nil, retErr
+		return nil, nil, retErr
 	}
 
-	return true, p, types.ToItem(b, repo, u), nil
+	return p, types.ToItem(b, repo, u), nil
 }
 
 // getPRNumberFromBuild is a helper function to

api/build/create.go

@@ -5,6 +5,7 @@ package build
 import (
 	"fmt"
 	"net/http"
+	"strings"
 
 	"github.com/gin-gonic/gin"
 	"github.com/go-vela/server/compiler"
@@ -16,6 +17,7 @@ import (
 	"github.com/go-vela/server/scm"
 	"github.com/go-vela/server/util"
 	"github.com/go-vela/types"
+	"github.com/go-vela/types/constants"
 	"github.com/go-vela/types/library"
 	"github.com/sirupsen/logrus"
 )
@@ -120,7 +122,7 @@ func CreateBuild(c *gin.Context) {
 		Retries:  1,
 	}
 
-	_, _, item, err := CompileAndPublish(
+	_, item, err := CompileAndPublish(
 		c,
 		config,
 		database.FromContext(c),
@@ -129,6 +131,13 @@ func CreateBuild(c *gin.Context) {
 		queue.FromContext(c),
 	)
 
+	// check if build was skipped
+	if err != nil && strings.EqualFold(item.Build.GetStatus(), constants.StatusSkipped) {
+		c.JSON(http.StatusOK, err.Error())
+
+		return
+	}
+
 	// error handling done in CompileAndPublish
 	if err != nil {
 		return

api/build/plan.go

@@ -10,6 +10,7 @@ import (
 	"github.com/go-vela/server/api/service"
 	"github.com/go-vela/server/api/step"
 	"github.com/go-vela/server/database"
+	"github.com/go-vela/server/scm"
 	"github.com/go-vela/types/library"
 	"github.com/go-vela/types/pipeline"
 )
@@ -19,7 +20,7 @@ import (
 // and services, for the build in the configured backend.
 // TODO:
 // - return build and error.
-func PlanBuild(ctx context.Context, database database.Interface, p *pipeline.Build, b *library.Build, r *library.Repo) error {
+func PlanBuild(ctx context.Context, database database.Interface, scm scm.Service, p *pipeline.Build, b *library.Build, r *library.Repo) error {
 	// update fields in build object
 	b.SetCreated(time.Now().UTC().Unix())
 
@@ -49,7 +50,7 @@ func PlanBuild(ctx context.Context, database database.Interface, p *pipeline.Bui
 	}
 
 	// plan all steps for the build
-	steps, err := step.PlanSteps(ctx, database, p, b)
+	steps, err := step.PlanSteps(ctx, database, scm, p, b, r)
 	if err != nil {
 		// clean up the objects from the pipeline in the database
 		CleanBuild(ctx, database, b, services, steps, err)

api/build/restart.go

@@ -119,7 +119,7 @@ func RestartBuild(c *gin.Context) {
 	}
 
 	// generate queue items
-	_, _, item, err := CompileAndPublish(
+	_, item, err := CompileAndPublish(
 		c,
 		config,
 		database.FromContext(c),
@@ -128,7 +128,7 @@ func RestartBuild(c *gin.Context) {
 		queue.FromContext(c),
 	)
 
-	// error handling done in GenerateQueueItems
+	// error handling done in CompileAndPublish
 	if err != nil {
 		return
 	}

api/step/plan.go

@@ -8,15 +8,17 @@ import (
 	"time"
 
 	"github.com/go-vela/server/database"
+	"github.com/go-vela/server/scm"
 	"github.com/go-vela/types/constants"
 	"github.com/go-vela/types/library"
 	"github.com/go-vela/types/pipeline"
+	"github.com/sirupsen/logrus"
 )
 
 // PlanSteps is a helper function to plan all steps
 // in the build for execution. This creates the steps
 // for the build in the configured backend.
-func PlanSteps(ctx context.Context, database database.Interface, p *pipeline.Build, b *library.Build) ([]*library.Step, error) {
+func PlanSteps(ctx context.Context, database database.Interface, scm scm.Service, p *pipeline.Build, b *library.Build, repo *library.Repo) ([]*library.Step, error) {
 	// variable to store planned steps
 	steps := []*library.Step{}
 
@@ -25,7 +27,7 @@ func PlanSteps(ctx context.Context, database database.Interface, p *pipeline.Bui
 		// iterate through all steps for each pipeline stage
 		for _, step := range stage.Steps {
 			// create the step object
-			s, err := planStep(ctx, database, b, step, stage.Name)
+			s, err := planStep(ctx, database, scm, b, step, repo, stage.Name)
 			if err != nil {
 				return steps, err
 			}
@@ -36,7 +38,7 @@ func PlanSteps(ctx context.Context, database database.Interface, p *pipeline.Bui
 
 	// iterate through all pipeline steps
 	for _, step := range p.Steps {
-		s, err := planStep(ctx, database, b, step, "")
+		s, err := planStep(ctx, database, scm, b, step, repo, "")
 		if err != nil {
 			return steps, err
 		}
@@ -47,7 +49,7 @@ func PlanSteps(ctx context.Context, database database.Interface, p *pipeline.Bui
 	return steps, nil
 }
 
-func planStep(ctx context.Context, database database.Interface, b *library.Build, c *pipeline.Container, stage string) (*library.Step, error) {
+func planStep(ctx context.Context, database database.Interface, scm scm.Service, b *library.Build, c *pipeline.Container, repo *library.Repo, stage string) (*library.Step, error) {
 	// create the step object
 	s := new(library.Step)
 	s.SetBuildID(b.GetID())
@@ -57,6 +59,7 @@ func planStep(ctx context.Context, database database.Interface, b *library.Build
 	s.SetImage(c.Image)
 	s.SetStage(stage)
 	s.SetStatus(constants.StatusPending)
+	s.SetReportAs(c.ReportAs)
 	s.SetCreated(time.Now().UTC().Unix())
 
 	// send API call to create the step
@@ -86,5 +89,19 @@ func planStep(ctx context.Context, database database.Interface, b *library.Build
 		return nil, fmt.Errorf("unable to create logs for step %s: %w", s.GetName(), err)
 	}
 
+	if len(s.GetReportAs()) > 0 {
+		// send API call to capture the repo owner
+		u, err := database.GetUser(ctx, repo.GetUserID())
+		if err != nil {
+			logrus.Errorf("unable to get owner for build: %v", err)
+		}
+
+		// send API call to set the status on the commit
+		err = scm.StepStatus(ctx, u, b, s, repo.GetOrg(), repo.GetName())
+		if err != nil {
+			logrus.Errorf("unable to set commit status for build: %v", err)
+		}
+	}
+
 	return s, nil
 }