From dcd2826277580fa911168222d8d1c1ac251d4059 Mon Sep 17 00:00:00 2001 From: mrinalirao Date: Mon, 16 Jan 2023 11:10:53 +1100 Subject: [PATCH] refactor runTaskStage func & use multierrors lib --- .../backend_taskStage_policyEvaluation.go | 2 +- ...backend_taskStage_policyEvaluation_test.go | 2 +- internal/cloud/backend_taskStages.go | 89 +++++++++---------- internal/cloud/errors.go | 44 --------- 4 files changed, 42 insertions(+), 95 deletions(-) diff --git a/internal/cloud/backend_taskStage_policyEvaluation.go b/internal/cloud/backend_taskStage_policyEvaluation.go index eeba9591a8..16f4c2e504 100644 --- a/internal/cloud/backend_taskStage_policyEvaluation.go +++ b/internal/cloud/backend_taskStage_policyEvaluation.go @@ -104,7 +104,7 @@ func (pes *policyEvaluationSummarizer) taskStageWithPolicyEvaluation(context *In // Currently only one policy evaluation supported : OPA for _, polEvaluation := range policyEvaluation { if polEvaluation.Status == tfe.PolicyEvaluationPassed { - message = "[dim] This result means that all OPA policies passed and the protected behaviour is allowed" + message = "[dim] This result means that all OPA policies passed and the protected behavior is allowed" result = fmt.Sprintf("[green]%s", strings.ToUpper(string(tfe.PolicyEvaluationPassed))) if polEvaluation.ResultCount.AdvisoryFailed > 0 { result += " (with advisory)" diff --git a/internal/cloud/backend_taskStage_policyEvaluation_test.go b/internal/cloud/backend_taskStage_policyEvaluation_test.go index 7ea54ec9e9..46218396d1 100644 --- a/internal/cloud/backend_taskStage_policyEvaluation_test.go +++ b/internal/cloud/backend_taskStage_policyEvaluation_test.go @@ -30,7 +30,7 @@ func TestCloud_runTaskStageWithPolicyEvaluation(t *testing.T) { }, writer: writer, context: integrationContext, - expectedOutputs: []string{"│ [bold]OPA Policy Evaluation\n\n│ [bold]→→ Overall Result: [green]PASSED\n│ [dim] This result means that all OPA policies passed and the protected behaviour is allowed\n│ 1 policies evaluated\n\n│ → Policy set 1: [bold]policy-set-that-passes (1)\n│ ↳ Policy name: [bold]policy-pass\n│ | [green][bold]✓ Passed\n│ | [dim]This policy will pass\n"}, + expectedOutputs: []string{"│ [bold]OPA Policy Evaluation\n\n│ [bold]→→ Overall Result: [green]PASSED\n│ [dim] This result means that all OPA policies passed and the protected behavior is allowed\n│ 1 policies evaluated\n\n│ → Policy set 1: [bold]policy-set-that-passes (1)\n│ ↳ Policy name: [bold]policy-pass\n│ | [green][bold]✓ Passed\n│ | [dim]This policy will pass\n"}, isError: false, }, "mandatory-failed": { diff --git a/internal/cloud/backend_taskStages.go b/internal/cloud/backend_taskStages.go index 72f62f7ba3..36dab7629a 100644 --- a/internal/cloud/backend_taskStages.go +++ b/internal/cloud/backend_taskStages.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" + "github.com/hashicorp/go-multierror" tfe "github.com/hashicorp/go-tfe" "github.com/hashicorp/terraform/internal/terraform" ) @@ -66,7 +67,7 @@ func (b *Cloud) getTaskStageWithAllOptions(ctx *IntegrationContext, stageID stri } func (b *Cloud) runTaskStage(ctx *IntegrationContext, output IntegrationOutputWriter, stageID string) error { - var errs multiErrors + var errs *multierror.Error // Create our summarizers summarizers := make([]taskStageSummarizer, 0) @@ -98,60 +99,33 @@ func (b *Cloud) runTaskStage(ctx *IntegrationContext, output IntegrationOutputWr return true, nil // Note: Terminal statuses need to print out one last time just in case case tfe.TaskStageRunning, tfe.TaskStagePassed: - for _, s := range summarizers { - cont, msg, err := s.Summarize(ctx, output, stage) - if err != nil { - errs.Append(err) - break - } - if !cont { - continue - } - - // cont is true and we must continue to poll - if msg != nil { - output.OutputElapsed(*msg, len(*msg)) // Up to 2 digits are allowed by the max message allocation - } + ok, e := processSummarizers(ctx, output, stage, summarizers, errs) + if e != nil { + errs = e + } + if ok { return true, nil } case tfe.TaskStageCanceled, tfe.TaskStageErrored, tfe.TaskStageFailed: - for _, s := range summarizers { - cont, msg, err := s.Summarize(ctx, output, stage) - if err != nil { - errs.Append(err) - break - } - - if !cont { - continue - } - - // cont is true and we must continue to poll - if msg != nil { - output.OutputElapsed(*msg, len(*msg)) // Up to 2 digits are allowed by the max message allocation - } + ok, e := processSummarizers(ctx, output, stage, summarizers, errs) + if e != nil { + errs = e + } + if ok { return true, nil } return false, fmt.Errorf("Task Stage %s.", stage.Status) case tfe.TaskStageAwaitingOverride: - for _, s := range summarizers { - cont, msg, err := s.Summarize(ctx, output, stage) - if err != nil { - errs.Append(err) - break - } - if !cont { - continue - } - // cont is true and we must continue to poll - if msg != nil { - output.OutputElapsed(*msg, len(*msg)) // Up to 2 digits are allowed by the max message allocation - } + ok, e := processSummarizers(ctx, output, stage, summarizers, errs) + if e != nil { + errs = e + } + if ok { return true, nil } cont, err := b.processStageOverrides(ctx, output, stage.ID) if err != nil { - errs.Append(err) + errs = multierror.Append(errs, err) } else { return cont, nil } @@ -160,14 +134,31 @@ func (b *Cloud) runTaskStage(ctx *IntegrationContext, output IntegrationOutputWr default: return false, fmt.Errorf("Invalid Task stage status: %s ", stage.Status) } - - if len(errs) > 0 { - return false, errs.Err() - } - return false, nil + return false, errs.ErrorOrNil() }) } +func processSummarizers(ctx *IntegrationContext, output IntegrationOutputWriter, stage *tfe.TaskStage, summarizers []taskStageSummarizer, errs *multierror.Error) (bool, *multierror.Error) { + for _, s := range summarizers { + cont, msg, err := s.Summarize(ctx, output, stage) + if err != nil { + errs = multierror.Append(errs, err) + break + } + + if !cont { + continue + } + + // cont is true and we must continue to poll + if msg != nil { + output.OutputElapsed(*msg, len(*msg)) // Up to 2 digits are allowed by the max message allocation + } + return true, nil + } + return false, errs +} + func (b *Cloud) processStageOverrides(context *IntegrationContext, output IntegrationOutputWriter, taskStageID string) (bool, error) { opts := &terraform.InputOpts{ Id: fmt.Sprintf("%c%c [bold]Override", Arrow, Arrow), diff --git a/internal/cloud/errors.go b/internal/cloud/errors.go index 193db30261..cf668516f3 100644 --- a/internal/cloud/errors.go +++ b/internal/cloud/errors.go @@ -58,47 +58,3 @@ func incompatibleWorkspaceTerraformVersion(message string, ignoreVersionConflict description := strings.TrimSpace(fmt.Sprintf("%s\n\n%s", message, suggestion)) return tfdiags.Sourceless(severity, "Incompatible Terraform version", description) } - -type multiErrors []error - -// Append errs to e only if the individual error in errs is not nil -// -// If any of the errs is itself multiErrors, each individual error in errs is appended. -func (e *multiErrors) Append(errs ...error) { - for _, err := range errs { - if err == nil { - continue - } - if errs, ok := err.(multiErrors); ok { - *e = append(*e, errs...) - } else { - *e = append(*e, err) - } - } -} - -// multiErrors returns an error string by joining -// all of its nonnil errors with colon separator. -func (e multiErrors) Error() string { - if len(e) == 0 { - return "" - } - es := make([]string, 0, len(e)) - for _, err := range e { - if err != nil { - es = append(es, err.Error()) - } - } - return strings.Join(es, ":") -} - -// Err returns e as an error or returns nil if no errors were collected. -func (e multiErrors) Err() error { - // Only return self if we have at least one nonnil error. - for _, err := range e { - if err != nil { - return e - } - } - return nil -}