Skip to content

Commit

Permalink
fix: allow CI pipeline to fail if the Change Request changes scm-engi…
Browse files Browse the repository at this point in the history
…ne configuration file (#80)

* fix: allow CI pipeline to fail if the Change Request changes scm-engine configuration file

otherwise, continue to fail open

* fix: MergeRequestIDUint() possible overflow
  • Loading branch information
jippi authored Sep 10, 2024
1 parent 5e4b5df commit dd581f2
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 8 deletions.
8 changes: 7 additions & 1 deletion cmd/shared.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,14 @@ func ProcessMR(ctx context.Context, client scm.Client, cfg *config.Config, event
// Track where we grab the configuration file from
ctx = slogctx.With(ctx, slog.String("config_source_branch", "merge_request_branch"))

// Should we allow failing the CI pipeline?
allowPipelineFailure := false

defer state.LockForProcessing(ctx)()

// Stop the pipeline when we leave this func
defer func() {
if stopErr := client.Stop(ctx, err); stopErr != nil {
if stopErr := client.Stop(ctx, err, allowPipelineFailure); stopErr != nil {
slogctx.Error(ctx, "Failed to update pipeline", slog.Any("error", stopErr))
}
}()
Expand All @@ -73,6 +76,9 @@ func ProcessMR(ctx context.Context, client scm.Client, cfg *config.Config, event
return nil
}

// Check if we are allowed to fail the CI pipeline
allowPipelineFailure = evalContext.AllowPipelineFailure(ctx)

//
// (Optional) Download the .scm-engine.yml configuration file from the GitLab HTTP API
//
Expand Down
2 changes: 1 addition & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
slogctx "github.com/veqryn/slog-context"
)

// nolint: gochecknoglobals
//nolint:gochecknoglobals
var (
commit = "unknown"
date = "unknown"
Expand Down
2 changes: 1 addition & 1 deletion pkg/scm/github/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func (client *Client) Start(ctx context.Context) error {
}

// Stop pipeline
func (client *Client) Stop(ctx context.Context, err error) error {
func (client *Client) Stop(ctx context.Context, err error, allowPipelineFailure bool) error {
return nil
}

Expand Down
4 changes: 4 additions & 0 deletions pkg/scm/github/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,3 +109,7 @@ func (c *Context) HasExecutedActionGroup(name string) bool {

return ok
}

func (c *Context) AllowPipelineFailure(ctx context.Context) bool {
return len(c.PullRequest.findModifiedFiles(state.ConfigFilePath(ctx))) == 1
}
7 changes: 5 additions & 2 deletions pkg/scm/gitlab/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ func (client *Client) Start(ctx context.Context) error {
}

// Stop pipeline
func (client *Client) Stop(ctx context.Context, evalError error) error {
func (client *Client) Stop(ctx context.Context, evalError error, allowPipelineFailure bool) error {
ok, pattern := state.ShouldUpdatePipeline(ctx)
if !ok {
return nil
Expand All @@ -210,7 +210,10 @@ func (client *Client) Stop(ctx context.Context, evalError error) error {
)

if evalError != nil {
status = go_gitlab.Success
if allowPipelineFailure {
status = go_gitlab.Failed
}

message = evalError.Error()
}

Expand Down
9 changes: 9 additions & 0 deletions pkg/scm/gitlab/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,15 @@ func (c *Context) CanUseConfigurationFileFromChangeRequest(ctx context.Context)
return true
}

// AllowPipelineFailure controls if the CI pipeline are allowed to fail
//
// We allow the pipeline to fail with an error if the SCM-Engine configuration file
// is changed within the merge request, effectively allowing us to lint the configuration
// file when changing it but failing "open" in all other cases.
func (c *Context) AllowPipelineFailure(ctx context.Context) bool {
return len(c.MergeRequest.findModifiedFiles(state.ConfigFilePath(ctx))) == 1
}

func (c *Context) TrackActionGroupExecution(group string) {
// Ungrouped actions shouldn't be tracked
if len(group) == 0 {
Expand Down
3 changes: 2 additions & 1 deletion pkg/scm/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ type Client interface {
Labels() LabelClient
MergeRequests() MergeRequestClient
Start(ctx context.Context) error
Stop(ctx context.Context, err error) error
Stop(ctx context.Context, err error, allowPipelineFailure bool) error
}

type LabelClient interface {
Expand All @@ -29,6 +29,7 @@ type MergeRequestClient interface {
}

type EvalContext interface {
AllowPipelineFailure(ctx context.Context) bool
CanUseConfigurationFileFromChangeRequest(ctx context.Context) bool
GetDescription() string
HasExecutedActionGroup(name string) bool
Expand Down
4 changes: 2 additions & 2 deletions pkg/state/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,10 @@ func MergeRequestIDInt(ctx context.Context) int {
func MergeRequestIDUint(ctx context.Context) uint64 {
val := MergeRequestID(ctx)

number, err := strconv.Atoi(val)
number, err := strconv.ParseUint(val, 10, 64)
if err != nil {
panic(err)
}

return uint64(number)
return number
}

0 comments on commit dd581f2

Please sign in to comment.