Skip to content

Commit eac4651

Browse files
committed
refactoring all the strategies
1 parent 5a33821 commit eac4651

File tree

14 files changed

+208
-176
lines changed

14 files changed

+208
-176
lines changed

cli/cmd/digger/main_test.go

+10-4
Original file line numberDiff line numberDiff line change
@@ -895,8 +895,11 @@ func TestGitHubNewPullRequestContext(t *testing.T) {
895895
impactedProjects, requestedProject, prNumber, err := dggithub.ProcessGitHubEvent(ghEvent, &diggerConfig, &prManager)
896896

897897
reporter := &reporting.CiReporter{
898-
CiService: &prManager,
899-
PrNumber: prNumber,
898+
CiService: &prManager,
899+
PrNumber: prNumber,
900+
IsSupportMarkdown: true,
901+
IsSuppressed: false,
902+
ReportStrategy: reporting.NewMultipleCommentsStrategy(),
900903
}
901904

902905
event := context.Event.(github.PullRequestEvent)
@@ -923,8 +926,11 @@ func TestGitHubNewCommentContext(t *testing.T) {
923926
planStorage := storage.MockPlanStorage{}
924927
impactedProjects, requestedProject, prNumber, err := dggithub.ProcessGitHubEvent(ghEvent, &diggerConfig, &prManager)
925928
reporter := &reporting.CiReporter{
926-
CiService: &prManager,
927-
PrNumber: prNumber,
929+
CiService: &prManager,
930+
PrNumber: prNumber,
931+
ReportStrategy: reporting.NewMultipleCommentsStrategy(),
932+
IsSuppressed: false,
933+
IsSupportMarkdown: true,
928934
}
929935

930936
policyChecker := policy.MockPolicyChecker{}

cli/cmd/digger/root.go

+4-5
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414
"log"
1515
"net/http"
1616
"os"
17-
"time"
1817
)
1918

2019
type RunConfig struct {
@@ -93,11 +92,11 @@ func PreRun(cmd *cobra.Command, args []string) {
9392
//PolicyChecker = policy.NewPolicyChecker(hostName, orgName, token)
9493

9594
if os.Getenv("REPORTING_STRATEGY") == "comments_per_run" || os.Getenv("ACCUMULATE_PLANS") == "true" {
96-
ReportStrategy = &reporting.SingleCommentStrategy{
97-
TimeOfRun: time.Now(),
98-
}
95+
strategy := reporting.NewSingleCommentStrategy()
96+
ReportStrategy = &strategy
9997
} else {
100-
ReportStrategy = &reporting.MultipleCommentsStrategy{}
98+
strategy := reporting.NewMultipleCommentsStrategy()
99+
ReportStrategy = &strategy
101100
}
102101

103102
var err error

cli/pkg/digger/digger.go

+20-19
Original file line numberDiff line numberDiff line change
@@ -124,13 +124,14 @@ func RunJobs(jobs []orchestrator.Job, prService ci.PullRequestService, orgServic
124124
}
125125

126126
if allAppliesSuccess == true && reportFinalStatusToBackend == true {
127-
_, jobPrCommentUrl, err := reporter.Flush()
127+
_, jobPrCommentUrls, err := reporter.Flush()
128128
if err != nil {
129129
log.Printf("error while sending job comments %v", err)
130130
return false, false, fmt.Errorf("error while sending job comments %v", err)
131131
}
132132

133133
currentJob := jobs[0]
134+
jobPrCommentUrl := jobPrCommentUrls[0]
134135
repoNameForBackendReporting := strings.ReplaceAll(currentJob.Namespace, "/", "-")
135136
projectNameForBackendReporting := currentJob.ProjectName
136137
// TODO: handle the apply result summary as well to report it to backend. Possibly reporting changed resources as well
@@ -170,12 +171,12 @@ func RunJobs(jobs []orchestrator.Job, prService ci.PullRequestService, orgServic
170171
func reportPolicyError(projectName string, command string, requestedBy string, reporter reporting.Reporter) string {
171172
msg := fmt.Sprintf("User %s is not allowed to perform action: %s. Check your policies :x:", requestedBy, command)
172173
if reporter.SupportsMarkdown() {
173-
err := reporter.Report(msg, coreutils.AsCollapsibleComment(fmt.Sprintf("Policy violation for <b>%v - %v</b>", projectName, command), false))
174+
err := reporter.Report(projectName, msg, coreutils.AsCollapsibleComment(fmt.Sprintf("Policy violation for <b>%v - %v</b>", projectName, command), false))
174175
if err != nil {
175176
log.Printf("Error publishing comment: %v", err)
176177
}
177178
} else {
178-
err := reporter.Report(msg, coreutils.AsComment(fmt.Sprintf("Policy violation for %v - %v", projectName, command)))
179+
err := reporter.Report(projectName, msg, coreutils.AsComment(fmt.Sprintf("Policy violation for %v - %v", projectName, command)))
179180
if err != nil {
180181
log.Printf("Error publishing comment: %v", err)
181182
}
@@ -284,7 +285,7 @@ func run(command string, job orchestrator.Job, policyChecker policy.Checker, org
284285
return nil, msg, fmt.Errorf(msg)
285286
} else if planPerformed {
286287
if isNonEmptyPlan {
287-
reportTerraformPlanOutput(reporter, projectLock.LockId(), plan)
288+
reportTerraformPlanOutput(reporter, job.ProjectName, plan)
288289
planIsAllowed, messages, err := policyChecker.CheckPlanPolicy(SCMrepository, SCMOrganisation, job.ProjectName, job.ProjectDir, planJsonOutput)
289290
if err != nil {
290291
msg := fmt.Sprintf("Failed to validate plan. %v", err)
@@ -311,7 +312,7 @@ func run(command string, job orchestrator.Job, policyChecker policy.Checker, org
311312
preformattedMessaged = append(preformattedMessaged, fmt.Sprintf(" %v", message))
312313
}
313314
planReportMessage = planReportMessage + strings.Join(preformattedMessaged, "<br>")
314-
err = reporter.Report(planReportMessage, planPolicyFormatter)
315+
err = reporter.Report(job.ProjectName, planReportMessage, planPolicyFormatter)
315316

316317
if err != nil {
317318
log.Printf("Failed to report plan. %v", err)
@@ -320,14 +321,14 @@ func run(command string, job orchestrator.Job, policyChecker policy.Checker, org
320321
log.Printf(msg)
321322
return nil, msg, fmt.Errorf(msg)
322323
} else {
323-
err := reporter.Report("Terraform plan validation checks succeeded :white_check_mark:", planPolicyFormatter)
324+
err := reporter.Report(job.ProjectName, "Terraform plan validation checks succeeded :white_check_mark:", planPolicyFormatter)
324325
if err != nil {
325326
log.Printf("Failed to report plan. %v", err)
326327
}
327-
reportPlanSummary(reporter, planSummary)
328+
reportPlanSummary(job.ProjectName, reporter, planSummary)
328329
}
329330
} else {
330-
reportEmptyPlanOutput(reporter, projectLock.LockId())
331+
reportEmptyPlanOutput(job.ProjectName, reporter, projectLock.LockId())
331332
}
332333
err := prService.SetStatus(*job.PullRequestNumber, "success", job.ProjectName+"/plan")
333334
if err != nil {
@@ -370,7 +371,7 @@ func run(command string, job orchestrator.Job, policyChecker policy.Checker, org
370371
}
371372
log.Printf("PR status, mergeable: %v, merged: %v and skipMergeCheck %v\n", isMergeable, isMerged, job.SkipMergeCheck)
372373
if !isMergeable && !isMerged && !job.SkipMergeCheck {
373-
comment := reportApplyMergeabilityError(reporter)
374+
comment := reportApplyMergeabilityError(job.ProjectName, reporter)
374375
prService.SetStatus(*job.PullRequestNumber, "failure", job.ProjectName+"/apply")
375376

376377
return nil, comment, fmt.Errorf(comment)
@@ -491,25 +492,25 @@ func run(command string, job orchestrator.Job, policyChecker policy.Checker, org
491492
return &execution.DiggerExecutorResult{}, "", nil
492493
}
493494

494-
func reportApplyMergeabilityError(reporter reporting.Reporter) string {
495+
func reportApplyMergeabilityError(projectName string, reporter reporting.Reporter) string {
495496
comment := "cannot perform Apply since the PR is not currently mergeable"
496497
log.Println(comment)
497498

498499
if reporter.SupportsMarkdown() {
499-
err := reporter.Report(comment, coreutils.AsCollapsibleComment("Apply error", false))
500+
err := reporter.Report(projectName, comment, coreutils.AsCollapsibleComment("Apply error", false))
500501
if err != nil {
501502
log.Printf("error publishing comment: %v\n", err)
502503
}
503504
} else {
504-
err := reporter.Report(comment, coreutils.AsComment("Apply error"))
505+
err := reporter.Report(projectName, comment, coreutils.AsComment("Apply error"))
505506
if err != nil {
506507
log.Printf("error publishing comment: %v\n", err)
507508
}
508509
}
509510
return comment
510511
}
511512

512-
func reportTerraformPlanOutput(reporter reporting.Reporter, projectId string, plan string) {
513+
func reportTerraformPlanOutput(reporter reporting.Reporter, projectName string, plan string) {
513514
var formatter func(string) string
514515

515516
if reporter.SupportsMarkdown() {
@@ -518,13 +519,13 @@ func reportTerraformPlanOutput(reporter reporting.Reporter, projectId string, pl
518519
formatter = coreutils.GetTerraformOutputAsComment("Plan output")
519520
}
520521

521-
err := reporter.Report(plan, formatter)
522+
err := reporter.Report(projectName, plan, formatter)
522523
if err != nil {
523524
log.Printf("Failed to report plan. %v", err)
524525
}
525526
}
526527

527-
func reportPlanSummary(reporter reporting.Reporter, summary string) {
528+
func reportPlanSummary(projectName string, reporter reporting.Reporter, summary string) {
528529
var formatter func(string) string
529530

530531
if reporter.SupportsMarkdown() {
@@ -533,19 +534,19 @@ func reportPlanSummary(reporter reporting.Reporter, summary string) {
533534
formatter = coreutils.AsComment("Plan summary")
534535
}
535536

536-
err := reporter.Report("\n"+summary, formatter)
537+
err := reporter.Report(projectName, "\n"+summary, formatter)
537538
if err != nil {
538539
log.Printf("Failed to report plan summary. %v", err)
539540
}
540541
}
541542

542-
func reportEmptyPlanOutput(reporter reporting.Reporter, projectId string) {
543+
func reportEmptyPlanOutput(projectName string, reporter reporting.Reporter, projectId string) {
543544
identityFormatter := func(comment string) string {
544545
return comment
545546
}
546-
err := reporter.Report("→ No changes in terraform output for "+projectId, identityFormatter)
547+
err := reporter.Report(projectName, "→ No changes in terraform output for "+projectId, identityFormatter)
547548
// suppress the comment (if reporter is suppressible)
548-
reporter.Suppress()
549+
reporter.Suppress("")
549550
if err != nil {
550551
log.Printf("Failed to report plan. %v", err)
551552
}

cli/pkg/digger/digger_test.go

+4-2
Original file line numberDiff line numberDiff line change
@@ -247,10 +247,11 @@ func TestCorrectCommandExecutionWhenApplying(t *testing.T) {
247247
prManager := &MockPRManager{}
248248
lock := &MockProjectLock{}
249249
planStorage := &MockPlanStorage{}
250+
strategy := reporting.NewMultipleCommentsStrategy()
250251
reporter := &reporting.CiReporter{
251252
CiService: prManager,
252253
PrNumber: 1,
253-
ReportStrategy: &reporting.MultipleCommentsStrategy{},
254+
ReportStrategy: &strategy,
254255
IsSupportMarkdown: true,
255256
}
256257
planPathProvider := &MockPlanPathProvider{}
@@ -297,10 +298,11 @@ func TestCorrectCommandExecutionWhenDestroying(t *testing.T) {
297298
prManager := &MockPRManager{}
298299
lock := &MockProjectLock{}
299300
planStorage := &MockPlanStorage{}
301+
strategy := reporting.NewMultipleCommentsStrategy()
300302
reporter := &reporting.CiReporter{
301303
CiService: prManager,
302304
PrNumber: 1,
303-
ReportStrategy: &reporting.MultipleCommentsStrategy{},
305+
ReportStrategy: &strategy,
304306
}
305307
planPathProvider := &MockPlanPathProvider{}
306308
executor := execution.DiggerExecutor{

cli/pkg/integration/integration_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,11 @@ func getProjectLockForTests() (error, *locking.PullRequestLock) {
4444
repositoryName := "test_dynamodb_lock"
4545
ghToken := "token"
4646
githubPrService, _ := dg_github.GithubServiceProviderBasic{}.NewService(ghToken, repositoryName, repoOwner)
47+
strategy := reporting.NewMultipleCommentsStrategy()
4748
reporter := reporting.CiReporter{
4849
CiService: &githubPrService,
4950
PrNumber: 1,
50-
ReportStrategy: &reporting.MultipleCommentsStrategy{},
51+
ReportStrategy: &strategy,
5152
}
5253

5354
projectLock := &locking.PullRequestLock{

ee/cli/cmd/digger/root.go

+4-5
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414
"log"
1515
"net/http"
1616
"os"
17-
"time"
1817
)
1918

2019
type RunConfig struct {
@@ -87,11 +86,11 @@ func PreRun(cmd *cobra.Command, args []string) {
8786
BackendApi = NewBackendApi(hostName, token)
8887

8988
if os.Getenv("REPORTING_STRATEGY") == "comments_per_run" || os.Getenv("ACCUMULATE_PLANS") == "true" {
90-
ReportStrategy = &reporting.SingleCommentStrategy{
91-
TimeOfRun: time.Now(),
92-
}
89+
strategy := reporting.NewSingleCommentStrategy()
90+
ReportStrategy = &strategy
9391
} else {
94-
ReportStrategy = &reporting.MultipleCommentsStrategy{}
92+
strategy := reporting.NewMultipleCommentsStrategy()
93+
ReportStrategy = &strategy
9594
}
9695

9796
var err error

libs/comment_utils/reporting/core.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
package reporting
22

33
type Reporter interface {
4-
Report(report string, reportFormatting func(report string) string) (error error)
5-
Flush() (string, string, error)
6-
Suppress() error
4+
Report(projectName string, report string, reportFormatting func(report string) string) (error error)
5+
Flush() ([]string, []string, error)
6+
Suppress(projectName string) error
77
SupportsMarkdown() bool
88
}

libs/comment_utils/reporting/mock.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,16 @@ type MockReporter struct {
44
commands []string
55
}
66

7-
func (mockReporter *MockReporter) Report(report string, reportFormatting func(report string) string) error {
7+
func (mockReporter *MockReporter) Report(projectName string, report string, reportFormatting func(report string) string) error {
88
mockReporter.commands = append(mockReporter.commands, "Report")
99
return nil
1010
}
1111

12-
func (mockReporter *MockReporter) Flush() (string, string, error) {
13-
return "", "", nil
12+
func (mockReporter *MockReporter) Flush() ([]string, []string, error) {
13+
return []string{}, []string{}, nil
1414
}
1515

16-
func (mockReporter *MockReporter) Suppress() error {
16+
func (mockReporter *MockReporter) Suppress(string) error {
1717
return nil
1818
}
1919

libs/comment_utils/reporting/noop.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,18 @@ package reporting
22

33
type NoopReporter struct{}
44

5-
func (reporter NoopReporter) Report(report string, reportFormatting func(report string) string) error {
5+
func (reporter NoopReporter) Report(projectName string, report string, reportFormatting func(report string) string) error {
66
return nil
77
}
88

9-
func (reporter NoopReporter) Flush() (string, string, error) {
10-
return "", "", nil
9+
func (reporter NoopReporter) Flush() ([]string, []string, error) {
10+
return []string{}, []string{}, nil
1111
}
1212

1313
func (reporter NoopReporter) SupportsMarkdown() bool {
1414
return false
1515
}
1616

17-
func (reporter NoopReporter) Suppress() error {
17+
func (reporter NoopReporter) Suppress(string) error {
1818
return nil
1919
}

0 commit comments

Comments
 (0)