Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: support single comment summary with orchestrator mode #1850

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from
Prev Previous commit
Next Next commit
refactor strategies
  • Loading branch information
motatoes committed Dec 19, 2024
commit dfe91e9941e4745231084350cab6615bf799dfa5
4 changes: 0 additions & 4 deletions cli/cmd/digger/root.go
Original file line number Diff line number Diff line change
@@ -96,10 +96,6 @@ func PreRun(cmd *cobra.Command, args []string) {
ReportStrategy = &reporting.CommentPerRunStrategy{
TimeOfRun: time.Now(),
}
} else if os.Getenv("REPORTING_STRATEGY") == "latest_run_comment" {
ReportStrategy = &reporting.LatestRunCommentStrategy{
TimeOfRun: time.Now(),
}
} else {
ReportStrategy = &reporting.MultipleCommentsStrategy{}
}
18 changes: 9 additions & 9 deletions cli/pkg/digger/digger.go
Original file line number Diff line number Diff line change
@@ -170,12 +170,12 @@ func RunJobs(jobs []orchestrator.Job, prService ci.PullRequestService, orgServic
func reportPolicyError(projectName string, command string, requestedBy string, reporter reporting.Reporter) string {
msg := fmt.Sprintf("User %s is not allowed to perform action: %s. Check your policies :x:", requestedBy, command)
if reporter.SupportsMarkdown() {
_, _, err := reporter.Report(msg, coreutils.AsCollapsibleComment(fmt.Sprintf("Policy violation for <b>%v - %v</b>", projectName, command), false))
err := reporter.Report(msg, coreutils.AsCollapsibleComment(fmt.Sprintf("Policy violation for <b>%v - %v</b>", projectName, command), false))
if err != nil {
log.Printf("Error publishing comment: %v", err)
}
} else {
_, _, err := reporter.Report(msg, coreutils.AsComment(fmt.Sprintf("Policy violation for %v - %v", projectName, command)))
err := reporter.Report(msg, coreutils.AsComment(fmt.Sprintf("Policy violation for %v - %v", projectName, command)))
if err != nil {
log.Printf("Error publishing comment: %v", err)
}
@@ -311,7 +311,7 @@ func run(command string, job orchestrator.Job, policyChecker policy.Checker, org
preformattedMessaged = append(preformattedMessaged, fmt.Sprintf(" %v", message))
}
planReportMessage = planReportMessage + strings.Join(preformattedMessaged, "<br>")
_, _, err = reporter.Report(planReportMessage, planPolicyFormatter)
err = reporter.Report(planReportMessage, planPolicyFormatter)

if err != nil {
log.Printf("Failed to report plan. %v", err)
@@ -320,7 +320,7 @@ func run(command string, job orchestrator.Job, policyChecker policy.Checker, org
log.Printf(msg)
return nil, msg, fmt.Errorf(msg)
} else {
_, _, err := reporter.Report("Terraform plan validation checks succeeded :white_check_mark:", planPolicyFormatter)
err := reporter.Report("Terraform plan validation checks succeeded :white_check_mark:", planPolicyFormatter)
if err != nil {
log.Printf("Failed to report plan. %v", err)
}
@@ -496,12 +496,12 @@ func reportApplyMergeabilityError(reporter reporting.Reporter) string {
log.Println(comment)

if reporter.SupportsMarkdown() {
_, _, err := reporter.Report(comment, coreutils.AsCollapsibleComment("Apply error", false))
err := reporter.Report(comment, coreutils.AsCollapsibleComment("Apply error", false))
if err != nil {
log.Printf("error publishing comment: %v\n", err)
}
} else {
_, _, err := reporter.Report(comment, coreutils.AsComment("Apply error"))
err := reporter.Report(comment, coreutils.AsComment("Apply error"))
if err != nil {
log.Printf("error publishing comment: %v\n", err)
}
@@ -518,7 +518,7 @@ func reportTerraformPlanOutput(reporter reporting.Reporter, projectId string, pl
formatter = coreutils.GetTerraformOutputAsComment("Plan output")
}

_, _, err := reporter.Report(plan, formatter)
err := reporter.Report(plan, formatter)
if err != nil {
log.Printf("Failed to report plan. %v", err)
}
@@ -533,7 +533,7 @@ func reportPlanSummary(reporter reporting.Reporter, summary string) {
formatter = coreutils.AsComment("Plan summary")
}

_, _, err := reporter.Report("\n"+summary, formatter)
err := reporter.Report("\n"+summary, formatter)
if err != nil {
log.Printf("Failed to report plan summary. %v", err)
}
@@ -543,7 +543,7 @@ func reportEmptyPlanOutput(reporter reporting.Reporter, projectId string) {
identityFormatter := func(comment string) string {
return comment
}
_, _, err := reporter.Report("→ No changes in terraform output for "+projectId, identityFormatter)
err := reporter.Report("→ No changes in terraform output for "+projectId, identityFormatter)
// suppress the comment (if reporter is suppressible)
reporter.Suppress()
if err != nil {
4 changes: 0 additions & 4 deletions ee/cli/cmd/digger/root.go
Original file line number Diff line number Diff line change
@@ -90,10 +90,6 @@ func PreRun(cmd *cobra.Command, args []string) {
ReportStrategy = &reporting.CommentPerRunStrategy{
TimeOfRun: time.Now(),
}
} else if os.Getenv("REPORTING_STRATEGY") == "latest_run_comment" {
ReportStrategy = &reporting.LatestRunCommentStrategy{
TimeOfRun: time.Now(),
}
} else {
ReportStrategy = &reporting.MultipleCommentsStrategy{}
}
2 changes: 1 addition & 1 deletion libs/comment_utils/reporting/core.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package reporting

type Reporter interface {
Report(report string, reportFormatting func(report string) string) (commentId string, commentUrl string, error error)
Report(report string, reportFormatting func(report string) string) (error error)
Flush() (string, string, error)
Suppress() error
SupportsMarkdown() bool
4 changes: 2 additions & 2 deletions libs/comment_utils/reporting/mock.go
Original file line number Diff line number Diff line change
@@ -4,9 +4,9 @@ type MockReporter struct {
commands []string
}

func (mockReporter *MockReporter) Report(report string, reportFormatting func(report string) string) (string, string, error) {
func (mockReporter *MockReporter) Report(report string, reportFormatting func(report string) string) error {
mockReporter.commands = append(mockReporter.commands, "Report")
return "", "", nil
return nil
}

func (mockReporter *MockReporter) Flush() (string, string, error) {
4 changes: 2 additions & 2 deletions libs/comment_utils/reporting/noop.go
Original file line number Diff line number Diff line change
@@ -2,8 +2,8 @@ package reporting

type NoopReporter struct{}

func (reporter NoopReporter) Report(report string, reportFormatting func(report string) string) (string, string, error) {
return "", "", nil
func (reporter NoopReporter) Report(report string, reportFormatting func(report string) string) error {
return nil
}

func (reporter NoopReporter) Flush() (string, string, error) {
34 changes: 14 additions & 20 deletions libs/comment_utils/reporting/reporting.go
Original file line number Diff line number Diff line change
@@ -16,9 +16,9 @@ type CiReporter struct {
ReportStrategy ReportStrategy
}

func (ciReporter CiReporter) Report(report string, reportFormatting func(report string) string) (string, string, error) {
commentId, commentUrl, err := ciReporter.ReportStrategy.Report(ciReporter.CiService, ciReporter.PrNumber, report, reportFormatting, ciReporter.SupportsMarkdown())
return commentId, commentUrl, err
func (ciReporter CiReporter) Report(report string, reportFormatting func(report string) string) error {
_, _, err := ciReporter.ReportStrategy.Report(ciReporter.CiService, ciReporter.PrNumber, report, reportFormatting, ciReporter.SupportsMarkdown())
return err
}

func (ciReporter CiReporter) Flush() (string, string, error) {
@@ -35,9 +35,9 @@ func (ciReporter CiReporter) SupportsMarkdown() bool {

type StdOutReporter struct{}

func (reporter StdOutReporter) Report(report string, reportFormatting func(report string) string) (string, string, error) {
func (reporter StdOutReporter) Report(report string, reportFormatting func(report string) string) error {
log.Printf("Info: %v", report)
return "", "", nil
return nil
}

func (reporter StdOutReporter) Flush() (string, string, error) {
@@ -54,6 +54,7 @@ func (reporter StdOutReporter) Suppress() error {

type ReportStrategy interface {
Report(ciService ci.PullRequestService, PrNumber int, report string, reportFormatter func(report string) string, supportsCollapsibleComment bool) (commentId string, commentUrl string, error error)
Flush() (string, string, error)
}

type CommentPerRunStrategy struct {
@@ -77,6 +78,10 @@ func (strategy CommentPerRunStrategy) Report(ciService ci.PullRequestService, Pr
return commentId, commentUrl, err
}

func (s CommentPerRunStrategy) Flush() (string, string, error) {
return "", "", nil
}

func upsertComment(ciService ci.PullRequestService, PrNumber int, report string, reportFormatter func(report string) string, comments []ci.Comment, reportTitle string, supportsCollapsible bool) (string, string, error) {
report = reportFormatter(report)
commentIdForThisRun := ""
@@ -127,24 +132,13 @@ func upsertComment(ciService ci.PullRequestService, PrNumber int, report string,
return fmt.Sprintf("%v", commentIdForThisRun), commentUrl, nil
}

type LatestRunCommentStrategy struct {
TimeOfRun time.Time
}

func (strategy LatestRunCommentStrategy) Report(ciService ci.PullRequestService, PrNumber int, report string, reportFormatter func(report string) string, supportsCollapsibleComment bool) (string, string, error) {
comments, err := ciService.GetComments(PrNumber)
if err != nil {
return "", "", fmt.Errorf("error getting comments: %v", err)
}

reportTitle := "Digger latest run report"
commentId, commentUrl, err := upsertComment(ciService, PrNumber, report, reportFormatter, comments, reportTitle, supportsCollapsibleComment)
return commentId, commentUrl, err
}

type MultipleCommentsStrategy struct{}

func (strategy MultipleCommentsStrategy) Report(ciService ci.PullRequestService, PrNumber int, report string, reportFormatter func(report string) string, supportsCollapsibleComment bool) (string, string, error) {
_, err := ciService.PublishComment(PrNumber, reportFormatter(report))
return "", "", err
}

func (s MultipleCommentsStrategy) Flush() (string, string, error) {
return "", "", nil
}
8 changes: 7 additions & 1 deletion libs/comment_utils/reporting/utils.go
Original file line number Diff line number Diff line change
@@ -30,7 +30,13 @@ func PostInitialSourceComments(ghService ci.PullRequestService, prNumber int, im
CiService: ghService,
ReportStrategy: CommentPerRunStrategy{fmt.Sprintf("Report for location: %v", location), time.Now()},
}
commentId, _, err := reporter.Report("Comment Reporter", func(report string) string { return "" })
err := reporter.Report("Comment Reporter", func(report string) string { return "" })
if err != nil {
log.Printf("Error reporting source module comment: %v", err)
return nil, fmt.Errorf("error reporting source module comment: %v", err)
}

commentId, _, err := reporter.Flush()
if err != nil {
log.Printf("Error reporting source module comment: %v", err)
return nil, fmt.Errorf("error reporting source module comment: %v", err)
16 changes: 8 additions & 8 deletions libs/execution/execution.go
Original file line number Diff line number Diff line change
@@ -294,12 +294,12 @@ func (d DiggerExecutor) Plan() (*iac_utils.IacSummary, bool, bool, string, strin

func reportError(r reporting.Reporter, stderr string) {
if r.SupportsMarkdown() {
_, _, commentErr := r.Report(stderr, utils.AsCollapsibleComment("Error during init.", false))
commentErr := r.Report(stderr, utils.AsCollapsibleComment("Error during init.", false))
if commentErr != nil {
log.Printf("error publishing comment: %v", commentErr)
}
} else {
_, _, commentErr := r.Report(stderr, utils.AsComment("Error during init."))
commentErr := r.Report(stderr, utils.AsComment("Error during init."))
if commentErr != nil {
log.Printf("error publishing comment: %v", commentErr)
}
@@ -376,12 +376,12 @@ func (d DiggerExecutor) Apply() (*iac_utils.IacSummary, bool, string, error) {

func reportApplyError(r reporting.Reporter, err error) {
if r.SupportsMarkdown() {
_, _, commentErr := r.Report(err.Error(), utils.AsCollapsibleComment("Error during applying.", false))
commentErr := r.Report(err.Error(), utils.AsCollapsibleComment("Error during applying.", false))
if commentErr != nil {
log.Printf("error publishing comment: %v", err)
}
} else {
_, _, commentErr := r.Report(err.Error(), utils.AsComment("Error during applying."))
commentErr := r.Report(err.Error(), utils.AsComment("Error during applying."))
if commentErr != nil {
log.Printf("error publishing comment: %v", err)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix incorrect error variable in log message

There's a bug in the error logging where it uses the wrong error variable.

-			log.Printf("error publishing comment: %v", err)
+			log.Printf("error publishing apply error comment: %v", commentErr)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
commentErr := r.Report(err.Error(), utils.AsCollapsibleComment("Error during applying.", false))
if commentErr != nil {
log.Printf("error publishing comment: %v", err)
}
} else {
_, _, commentErr := r.Report(err.Error(), utils.AsComment("Error during applying."))
commentErr := r.Report(err.Error(), utils.AsComment("Error during applying."))
if commentErr != nil {
log.Printf("error publishing comment: %v", err)
}
commentErr := r.Report(err.Error(), utils.AsCollapsibleComment("Error during applying.", false))
if commentErr != nil {
log.Printf("error publishing apply error comment: %v", commentErr)
}
} else {
commentErr := r.Report(err.Error(), utils.AsComment("Error during applying."))
if commentErr != nil {
log.Printf("error publishing apply error comment: %v", commentErr)
}

@@ -396,20 +396,20 @@ func reportTerraformApplyOutput(r reporting.Reporter, projectId string, applyOut
formatter = utils.GetTerraformOutputAsComment("Apply output")
}

_, _, commentErr := r.Report(applyOutput, formatter)
commentErr := r.Report(applyOutput, formatter)
if commentErr != nil {
log.Printf("error publishing comment: %v", commentErr)
}
}

func reportTerraformError(r reporting.Reporter, stderr string) {
if r.SupportsMarkdown() {
_, _, commentErr := r.Report(stderr, utils.GetTerraformOutputAsCollapsibleComment("Error during init.", false))
commentErr := r.Report(stderr, utils.GetTerraformOutputAsCollapsibleComment("Error during init.", false))
if commentErr != nil {
log.Printf("error publishing comment: %v", commentErr)
}
} else {
_, _, commentErr := r.Report(stderr, utils.GetTerraformOutputAsComment("Error during init."))
commentErr := r.Report(stderr, utils.GetTerraformOutputAsComment("Error during init."))
if commentErr != nil {
log.Printf("error publishing comment: %v", commentErr)
}
@@ -428,7 +428,7 @@ func reportAdditionalOutput(r reporting.Reporter, projectId string) {
output, _ := os.ReadFile(diggerOutPath)
outputStr := string(output)
if len(outputStr) > 0 {
_, _, commentErr := r.Report(outputStr, formatter)
commentErr := r.Report(outputStr, formatter)
if commentErr != nil {
log.Printf("error publishing comment: %v", commentErr)
}
12 changes: 6 additions & 6 deletions libs/locking/locking.go
Original file line number Diff line number Diff line change
@@ -96,12 +96,12 @@ func (projectLock *PullRequestLock) Lock() (bool, error) {

func reportingLockingSuccess(r reporting.Reporter, comment string) {
if r.SupportsMarkdown() {
_, _, err := r.Report(comment, utils.AsCollapsibleComment("Locking successful", false))
err := r.Report(comment, utils.AsCollapsibleComment("Locking successful", false))
if err != nil {
log.Println("failed to publish comment: " + err.Error())
}
} else {
_, _, err := r.Report(comment, utils.AsComment("Locking successful"))
err := r.Report(comment, utils.AsComment("Locking successful"))
if err != nil {
log.Println("failed to publish comment: " + err.Error())
}
@@ -110,12 +110,12 @@ func reportingLockingSuccess(r reporting.Reporter, comment string) {

func reportLockingFailed(r reporting.Reporter, comment string) {
if r.SupportsMarkdown() {
_, _, err := r.Report(comment, utils.AsCollapsibleComment("Locking failed", false))
err := r.Report(comment, utils.AsCollapsibleComment("Locking failed", false))
if err != nil {
log.Println("failed to publish comment: " + err.Error())
}
} else {
_, _, err := r.Report(comment, utils.AsComment("Locking failed"))
err := r.Report(comment, utils.AsComment("Locking failed"))
if err != nil {
log.Println("failed to publish comment: " + err.Error())
}
@@ -183,12 +183,12 @@ func (projectLock *PullRequestLock) Unlock() (bool, error) {

func reportSuccessfulUnlocking(r reporting.Reporter, comment string) {
if r.SupportsMarkdown() {
_, _, err := r.Report(comment, utils.AsCollapsibleComment("Unlocking successful", false))
err := r.Report(comment, utils.AsCollapsibleComment("Unlocking successful", false))
if err != nil {
log.Println("failed to publish comment: " + err.Error())
}
} else {
_, _, err := r.Report(comment, utils.AsComment("Unlocking successful"))
err := r.Report(comment, utils.AsComment("Unlocking successful"))
if err != nil {
log.Println("failed to publish comment: " + err.Error())
}
11 changes: 0 additions & 11 deletions libs/spec/providers.go
Original file line number Diff line number Diff line change
@@ -121,17 +121,6 @@ func (r ReporterProvider) GetReporter(title string, reporterSpec ReporterSpec, c
Title: title,
TimeOfRun: time.Now(),
}
case "latest_run_comment":
return reporting.LatestRunCommentStrategy{
TimeOfRun: time.Now(),
}
case "always_same_comment":
return reporting.AlwaysSameCommentStrategy{
Title: title,
TimeOfRun: time.Now(),
CommentId: *reporterSpec.ReportCommentId,
}

default:
return reporting.MultipleCommentsStrategy{}
}
Loading
Oops, something went wrong.