Add PullRequestCommenter provider trait

This allows for providers to comment on pull requests

Signed-off-by: Juan Antonio Osorio <[email protected]>

internal/engine/actions/alert/alert.go

@@ -53,7 +53,7 @@ func NewRuleAlert(
 		if alertCfg.GetPullRequestComment() == nil {
 			return nil, fmt.Errorf("alert engine missing pull_request_review configuration")
 		}
-		client, err := provinfv1.As[provinfv1.GitHub](provider)
+		client, err := provinfv1.As[provinfv1.PullRequestCommenter](provider)
 		if err != nil {
 			zerolog.Ctx(ctx).Debug().Str("rule-type", ruletype.GetName()).
 				Msg("provider is not a GitHub provider. Silently skipping alerts.")

internal/engine/actions/alert/pull_request_comment/pull_request_comment.go

@@ -10,11 +10,8 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"math"
-	"strconv"
-	"time"
-
 	"github.com/google/go-github/v63/github"
+	"github.com/mindersec/minder/internal/entities/properties"
 	"github.com/rs/zerolog"
 	"google.golang.org/protobuf/reflect/protoreflect"
 
@@ -39,7 +36,7 @@ const (
 // Alert is the structure backing the noop alert
 type Alert struct {
 	actionType interfaces.ActionType
-	gh         provifv1.GitHub
+	commenter  provifv1.PullRequestCommenter
 	reviewCfg  *pb.RuleType_Definition_Alert_AlertTypePRComment
 	setting    models.ActionOpt
 }
@@ -54,26 +51,17 @@ type PrCommentTemplateParams struct {
 }
 
 type paramsPR struct {
-	Owner      string
-	Repo       string
-	CommitSha  string
-	Number     int
 	Comment    string
-	Metadata   *alertMetadata
+	props      *properties.Properties
+	Metadata   *provifv1.CommentResultMeta
 	prevStatus *db.ListRuleEvaluationsByProfileIdRow
 }
 
-type alertMetadata struct {
-	ReviewID       string     `json:"review_id,omitempty"`
-	SubmittedAt    *time.Time `json:"submitted_at,omitempty"`
-	PullRequestUrl *string    `json:"pull_request_url,omitempty"`
-}
-
 // NewPullRequestCommentAlert creates a new pull request comment alert action
 func NewPullRequestCommentAlert(
 	actionType interfaces.ActionType,
 	reviewCfg *pb.RuleType_Definition_Alert_AlertTypePRComment,
-	gh provifv1.GitHub,
+	gh provifv1.PullRequestCommenter,
 	setting models.ActionOpt,
 ) (*Alert, error) {
 	if actionType == "" {
@@ -82,7 +70,7 @@ func NewPullRequestCommentAlert(
 
 	return &Alert{
 		actionType: actionType,
-		gh:         gh,
+		commenter:  gh,
 		reviewCfg:  reviewCfg,
 		setting:    setting,
 	}, nil
@@ -134,68 +122,14 @@ func (alert *Alert) Do(
 }
 
 func (alert *Alert) run(ctx context.Context, params *paramsPR, cmd interfaces.ActionCmd) (json.RawMessage, error) {
-	logger := zerolog.Ctx(ctx)
-
 	// Process the command
 	switch cmd {
-	// Create a review
 	case interfaces.ActionCmdOn:
-		review := &github.PullRequestReviewRequest{
-			CommitID: github.String(params.CommitSha),
-			Event:    github.String("COMMENT"),
-			Body:     github.String(params.Comment),
-		}
-
-		r, err := alert.gh.CreateReview(
-			ctx,
-			params.Owner,
-			params.Repo,
-			params.Number,
-			review,
-		)
-		if err != nil {
-			return nil, fmt.Errorf("error creating PR review: %w, %w", err, enginerr.ErrActionFailed)
-		}
-
-		newMeta, err := json.Marshal(alertMetadata{
-			ReviewID:       strconv.FormatInt(r.GetID(), 10),
-			SubmittedAt:    r.SubmittedAt.GetTime(),
-			PullRequestUrl: r.PullRequestURL,
-		})
-		if err != nil {
-			return nil, fmt.Errorf("error marshalling alert metadata json: %w", err)
-		}
-
-		logger.Info().Int64("review_id", *r.ID).Msg("PR review created")
-		return newMeta, nil
-	// Dismiss the review
+		// Create a review
+		return alert.runDoReview(ctx, params)
 	case interfaces.ActionCmdOff:
-		if params.Metadata == nil {
-			// We cannot do anything without the PR review ID, so we assume that turning the alert off is a success
-			return nil, fmt.Errorf("no PR comment ID provided: %w", enginerr.ErrActionTurnedOff)
-		}
-
-		reviewID, err := strconv.ParseInt(params.Metadata.ReviewID, 10, 64)
-		if err != nil {
-			zerolog.Ctx(ctx).Error().Err(err).Str("review_id", params.Metadata.ReviewID).Msg("failed to parse review_id")
-			return nil, fmt.Errorf("no PR comment ID provided: %w, %w", err, enginerr.ErrActionTurnedOff)
-		}
-
-		_, err = alert.gh.DismissReview(ctx, params.Owner, params.Repo, params.Number, reviewID,
-			&github.PullRequestReviewDismissalRequest{
-				Message: github.String("Dismissed due to alert being turned off"),
-			})
-		if err != nil {
-			if errors.Is(err, enginerr.ErrNotFound) {
-				// There's no PR review with that ID anymore.
-				// We exit by stating that the action was turned off.
-				return nil, fmt.Errorf("PR comment already dismissed: %w, %w", err, enginerr.ErrActionTurnedOff)
-			}
-			return nil, fmt.Errorf("error dismissing PR comment: %w, %w", err, enginerr.ErrActionFailed)
-		}
-		logger.Info().Str("review_id", params.Metadata.ReviewID).Msg("PR comment dismissed")
-		// Success - return ErrActionTurnedOff to indicate the action was successful
-		return nil, fmt.Errorf("%s : %w", alert.Class(), enginerr.ErrActionTurnedOff)
+		// Dismiss the review
+		return runDismissReview(ctx, alert, params)
 	case interfaces.ActionCmdDoNothing:
 		// Return the previous alert status.
 		return alert.runDoNothing(ctx, params)
@@ -211,16 +145,16 @@ func (alert *Alert) runDry(ctx context.Context, params *paramsPR, cmd interfaces
 	switch cmd {
 	case interfaces.ActionCmdOn:
 		body := github.String(params.Comment)
-		logger.Info().Msgf("dry run: create a PR comment on PR %d in repo %s/%s with the following body: %s",
-			params.Number, params.Owner, params.Repo, *body)
+		logger.Info().Dict("properties", params.props.ToLogDict()).
+			Msgf("dry run: create a PR comment on PR with body: %s", *body)
 		return nil, nil
 	case interfaces.ActionCmdOff:
 		if params.Metadata == nil {
 			// We cannot do anything without the PR review ID, so we assume that turning the alert off is a success
 			return nil, fmt.Errorf("no PR comment ID provided: %w", enginerr.ErrActionTurnedOff)
 		}
-		logger.Info().Msgf("dry run: dismiss PR comment %s on PR PR %d in repo %s/%s", params.Metadata.ReviewID,
-			params.Number, params.Owner, params.Repo)
+		logger.Info().Dict("properties", params.props.ToLogDict()).
+			Msgf("dry run: dismiss PR comment on PR")
 	case interfaces.ActionCmdDoNothing:
 		// Return the previous alert status.
 		return alert.runDoNothing(ctx, params)
@@ -231,7 +165,7 @@ func (alert *Alert) runDry(ctx context.Context, params *paramsPR, cmd interfaces
 
 // runDoNothing returns the previous alert status
 func (_ *Alert) runDoNothing(ctx context.Context, params *paramsPR) (json.RawMessage, error) {
-	logger := zerolog.Ctx(ctx).With().Str("repo", params.Repo).Logger()
+	logger := zerolog.Ctx(ctx).With().Dict("properties", params.props.ToLogDict()).Logger()
 
 	logger.Debug().Msg("Running do nothing")
 
@@ -245,6 +179,49 @@ func (_ *Alert) runDoNothing(ctx context.Context, params *paramsPR) (json.RawMes
 	return nil, err
 }
 
+func (alert *Alert) runDoReview(ctx context.Context, params *paramsPR) (json.RawMessage, error) {
+	logger := zerolog.Ctx(ctx)
+
+	r, err := alert.commenter.CommentOnPullRequest(ctx, params.props, provifv1.PullRequestCommentInfo{
+		// TODO: We should add the header to identify the alert. We could use the
+		// rule type name.
+		Commit: params.props.GetProperty(properties.PullRequestCommitSHA).GetString(),
+		Body:   params.Comment,
+		// TODO: Determine the priority from the rule type severity
+	})
+	if err != nil {
+		return nil, fmt.Errorf("error creating PR review: %w, %w", err, enginerr.ErrActionFailed)
+	}
+	logger.Info().Str("review_id", r.ID).Msg("PR review created")
+
+	// serialize r to json
+	meta, err := json.Marshal(r)
+	if err != nil {
+		return nil, fmt.Errorf("error marshalling PR review metadata: %w", err)
+	}
+
+	return meta, nil
+}
+
+func runDismissReview(ctx context.Context, alert *Alert, params *paramsPR) (json.RawMessage, error) {
+	if params.Metadata == nil {
+		// We cannot do anything without the PR review ID, so we assume that turning the alert off is a success
+		return nil, fmt.Errorf("no PR comment ID provided: %w", enginerr.ErrActionTurnedOff)
+	}
+
+	err := alert.commenter.DismissPullRequestReview(ctx, params.props)
+	if err != nil {
+		if errors.Is(err, enginerr.ErrNotFound) {
+			// There's no PR review with that ID anymore.
+			// We exit by stating that the action was turned off.
+			return nil, fmt.Errorf("PR comment already dismissed: %w, %w", err, enginerr.ErrActionTurnedOff)
+		}
+		return nil, fmt.Errorf("error dismissing PR comment: %w, %w", err, enginerr.ErrActionFailed)
+	}
+	// Success - return ErrActionTurnedOff to indicate the action was successful
+	return nil, fmt.Errorf("%s : %w", alert.Class(), enginerr.ErrActionTurnedOff)
+}
+
 // getParamsForSecurityAdvisory extracts the details from the entity
 func (alert *Alert) getParamsForPRComment(
 	ctx context.Context,
@@ -253,19 +230,15 @@ func (alert *Alert) getParamsForPRComment(
 	metadata *json.RawMessage,
 ) (*paramsPR, error) {
 	logger := zerolog.Ctx(ctx)
-	result := &paramsPR{
-		prevStatus: params.GetEvalStatusFromDb(),
-		Owner:      pr.GetRepoOwner(),
-		Repo:       pr.GetRepoName(),
-		CommitSha:  pr.GetCommitSha(),
+	props, err := properties.NewProperties(pr.GetProperties().AsMap())
+	if err != nil {
+		return nil, fmt.Errorf("error creating properties: %w", err)
 	}
 
-	// The GitHub Go API takes an int32, but our proto stores an int64; make sure we don't overflow
-	// The PR number is an int in GitHub and Gitlab; in practice overflow will never happen.
-	if pr.Number > math.MaxInt {
-		return nil, fmt.Errorf("pr number is too large")
+	result := &paramsPR{
+		prevStatus: params.GetEvalStatusFromDb(),
+		props:      props,
 	}
-	result.Number = int(pr.Number)
 
 	commentTmpl, err := util.NewSafeHTMLTemplate(&alert.reviewCfg.ReviewMessage, "message")
 	if err != nil {
@@ -289,7 +262,7 @@ func (alert *Alert) getParamsForPRComment(
 
 	// Unmarshal the existing alert metadata, if any
 	if metadata != nil {
-		meta := &alertMetadata{}
+		meta := &provifv1.CommentResultMeta{}
 		err := json.Unmarshal(*metadata, meta)
 		if err != nil {
 			// There's nothing saved apparently, so no need to fail here, but do log the error

internal/engine/actions/alert/pull_request_comment/pull_request_comment_test.go

@@ -18,10 +18,10 @@ import (
 	enginerr "github.com/mindersec/minder/internal/engine/errors"
 	engif "github.com/mindersec/minder/internal/engine/interfaces"
 	pbinternal "github.com/mindersec/minder/internal/proto"
-	mockghclient "github.com/mindersec/minder/internal/providers/github/mock"
 	pb "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1"
 	"github.com/mindersec/minder/pkg/engine/v1/interfaces"
 	"github.com/mindersec/minder/pkg/profiles/models"
+	mockcommenter "github.com/mindersec/minder/pkg/providers/v1/mock"
 )
 
 var TestActionTypeValid engif.ActionType = "alert-test"
@@ -44,7 +44,7 @@ func TestPullRequestCommentAlert(t *testing.T) {
 		cmd              engif.ActionCmd
 		reviewMsg        string
 		inputMetadata    *json.RawMessage
-		mockSetup        func(*mockghclient.MockGitHub)
+		mockSetup        func(commenter *mockcommenter.MockPullRequestCommenter)
 		expectedErr      error
 		expectedMetadata json.RawMessage
 	}{
@@ -53,9 +53,9 @@ func TestPullRequestCommentAlert(t *testing.T) {
 			actionType: TestActionTypeValid,
 			reviewMsg:  "This is a constant review message",
 			cmd:        engif.ActionCmdOn,
-			mockSetup: func(mockGitHub *mockghclient.MockGitHub) {
+			mockSetup: func(mockGitHub *mockcommenter.MockPullRequestCommenter) {
 				mockGitHub.EXPECT().
-					CreateReview(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
+					CommentOnPullRequest(gomock.Any(), gomock.Any(), gomock.Any()).
 					Return(&github.PullRequestReview{ID: &reviewID}, nil)
 			},
 			expectedMetadata: json.RawMessage(fmt.Sprintf(`{"review_id":"%s"}`, reviewIDStr)),
@@ -65,9 +65,9 @@ func TestPullRequestCommentAlert(t *testing.T) {
 			actionType: TestActionTypeValid,
 			reviewMsg:  "{{ .EvalErrorDetails }}",
 			cmd:        engif.ActionCmdOn,
-			mockSetup: func(mockGitHub *mockghclient.MockGitHub) {
+			mockSetup: func(mockGitHub *mockcommenter.MockPullRequestCommenter) {
 				mockGitHub.EXPECT().
-					CreateReview(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.AssignableToTypeOf(&github.PullRequestReviewRequest{})).
+					CommentOnPullRequest(gomock.Any(), gomock.Any(), gomock.Any()).
 					DoAndReturn(validateReviewBodyAndReturn(evaluationFailureDetails, reviewID))
 			},
 			expectedMetadata: json.RawMessage(fmt.Sprintf(`{"review_id":"%s"}`, reviewIDStr)),
@@ -77,9 +77,9 @@ func TestPullRequestCommentAlert(t *testing.T) {
 			actionType: TestActionTypeValid,
 			reviewMsg:  "{{ .EvalResultOutput.ViolationMsg }}",
 			cmd:        engif.ActionCmdOn,
-			mockSetup: func(mockGitHub *mockghclient.MockGitHub) {
+			mockSetup: func(mockGitHub *mockcommenter.MockPullRequestCommenter) {
 				mockGitHub.EXPECT().
-					CreateReview(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.AssignableToTypeOf(&github.PullRequestReviewRequest{})).
+					CommentOnPullRequest(gomock.Any(), gomock.Any(), gomock.Any()).
 					DoAndReturn(validateReviewBodyAndReturn(violationMsg, reviewID))
 			},
 			expectedMetadata: json.RawMessage(fmt.Sprintf(`{"review_id":"%s"}`, reviewIDStr)),
@@ -89,9 +89,9 @@ func TestPullRequestCommentAlert(t *testing.T) {
 			actionType: TestActionTypeValid,
 			reviewMsg:  "This is a constant review message",
 			cmd:        engif.ActionCmdOn,
-			mockSetup: func(mockGitHub *mockghclient.MockGitHub) {
+			mockSetup: func(mockGitHub *mockcommenter.MockPullRequestCommenter) {
 				mockGitHub.EXPECT().
-					CreateReview(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
+					CommentOnPullRequest(gomock.Any(), gomock.Any(), gomock.Any()).
 					Return(nil, fmt.Errorf("failed to create PR comment"))
 			},
 			expectedErr: enginerr.ErrActionFailed,
@@ -102,9 +102,9 @@ func TestPullRequestCommentAlert(t *testing.T) {
 			reviewMsg:     "This is a constant review message",
 			cmd:           engif.ActionCmdOff,
 			inputMetadata: &successfulRunMetadata,
-			mockSetup: func(mockGitHub *mockghclient.MockGitHub) {
+			mockSetup: func(mockGitHub *mockcommenter.MockPullRequestCommenter) {
 				mockGitHub.EXPECT().
-					DismissReview(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
+					CommentOnPullRequest(gomock.Any(), gomock.Any(), gomock.Any()).
 					Return(&github.PullRequestReview{}, nil)
 			},
 			expectedErr: enginerr.ErrActionTurnedOff,
@@ -126,7 +126,7 @@ func TestPullRequestCommentAlert(t *testing.T) {
 				ReviewMessage: tt.reviewMsg,
 			}
 
-			mockClient := mockghclient.NewMockGitHub(ctrl)
+			mockClient := mockcommenter.NewMockPullRequestCommenter(ctrl)
 			tt.mockSetup(mockClient)
 
 			prCommentAlert, err := NewPullRequestCommentAlert(