From fc7b6b7f151911e3e7bf507dd3b855ca77e41dd7 Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Tue, 20 Sep 2022 15:35:56 +0800 Subject: [PATCH 1/4] Upgrade go-tfe to 1.10.0 This commit updates the go-tfe library to the latest version (1.10.0). This is needed by later commits for the pre-apply task stage. --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index a4256fb7329f..5f48176e829c 100644 --- a/go.mod +++ b/go.mod @@ -38,7 +38,7 @@ require ( github.com/hashicorp/go-multierror v1.1.1 github.com/hashicorp/go-plugin v1.4.3 github.com/hashicorp/go-retryablehttp v0.7.1 - github.com/hashicorp/go-tfe v1.9.0 + github.com/hashicorp/go-tfe v1.10.0 github.com/hashicorp/go-uuid v1.0.3 github.com/hashicorp/go-version v1.6.0 github.com/hashicorp/hcl v0.0.0-20170504190234-a4b07c25de5f diff --git a/go.sum b/go.sum index 05b473fce2bb..2690f8a9973a 100644 --- a/go.sum +++ b/go.sum @@ -368,8 +368,8 @@ github.com/hashicorp/go-slug v0.10.0/go.mod h1:Ib+IWBYfEfJGI1ZyXMGNbu2BU+aa3Dzu4 github.com/hashicorp/go-sockaddr v1.0.0 h1:GeH6tui99pF4NJgfnhp+L6+FfobzVW3Ah46sLo0ICXs= github.com/hashicorp/go-sockaddr v1.0.0/go.mod h1:7Xibr9yA9JjQq1JpNB2Vw7kxv8xerXegt+ozgdvDeDU= github.com/hashicorp/go-syslog v1.0.0/go.mod h1:qPfqrKkXGihmCqbJM2mZgkZGvKG1dFdvsLplgctolz4= -github.com/hashicorp/go-tfe v1.9.0 h1:jkmyo7WKNA7gZDegG5imndoC4sojWXhqMufO+KcHqrU= -github.com/hashicorp/go-tfe v1.9.0/go.mod h1:uSWi2sPw7tLrqNIiASid9j3SprbbkPSJ/2s3X0mMemg= +github.com/hashicorp/go-tfe v1.10.0 h1:mkEge/DSca8VQeBSAQbjEy8fWFHbrJA76M7dny5XlYc= +github.com/hashicorp/go-tfe v1.10.0/go.mod h1:uSWi2sPw7tLrqNIiASid9j3SprbbkPSJ/2s3X0mMemg= github.com/hashicorp/go-uuid v1.0.0/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro= github.com/hashicorp/go-uuid v1.0.1/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro= github.com/hashicorp/go-uuid v1.0.2/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro= From 18da2f021398a2f0c3b183aa12d1f10fabe31292 Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Tue, 20 Sep 2022 16:08:10 +0800 Subject: [PATCH 2/4] Refactor task stage retrieval in cloud backend Previously all of the logic to retrieve Task Stages was in the backend_plan.go file. This commit: * Moves the logic to the backend_taskStages.go file. * Replaces using an array to a map indexed by the Task Stage's stage. This makes it easier to find the relevant stage in the list and means the getTaskStageIDByName function is no longer required. --- internal/cloud/backend_plan.go | 36 ++++++---------------------- internal/cloud/backend_taskStages.go | 32 +++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 29 deletions(-) create mode 100644 internal/cloud/backend_taskStages.go diff --git a/internal/cloud/backend_plan.go b/internal/cloud/backend_plan.go index 2688d65c1288..0678d9360835 100644 --- a/internal/cloud/backend_plan.go +++ b/internal/cloud/backend_plan.go @@ -293,22 +293,13 @@ in order to capture the filesystem context the remote workspace expects: // Retrieve the run to get task stages. // Task Stages are calculated upfront so we only need to call this once for the run. - taskStages := make([]*tfe.TaskStage, 0) - result, err := b.client.Runs.ReadWithOptions(stopCtx, r.ID, &tfe.RunReadOptions{ - Include: []tfe.RunIncludeOpt{tfe.RunTaskStages}, - }) - if err == nil { - taskStages = result.TaskStages - } else { - // This error would be expected for older versions of TFE that do not allow - // fetching task_stages. - if !strings.HasSuffix(err.Error(), "Invalid include parameter") { - return r, generalError("Failed to retrieve run", err) - } + taskStages, err := b.runTaskStages(stopCtx, b.client, r.ID) + if err != nil { + return r, err } - if stageID := getTaskStageIDByName(taskStages, tfe.PrePlan); stageID != nil { - if err := b.waitTaskStage(stopCtx, cancelCtx, op, r, *stageID, "Pre-plan Tasks"); err != nil { + if stage, ok := taskStages[tfe.PrePlan]; ok { + if err := b.waitTaskStage(stopCtx, cancelCtx, op, r, stage.ID, "Pre-plan Tasks"); err != nil { return r, err } } @@ -357,8 +348,8 @@ in order to capture the filesystem context the remote workspace expects: // status of the run will be "errored", but there is still policy // information which should be shown. - if stageID := getTaskStageIDByName(taskStages, tfe.PostPlan); stageID != nil { - if err := b.waitTaskStage(stopCtx, cancelCtx, op, r, *stageID, "Post-plan Tasks"); err != nil { + if stage, ok := taskStages[tfe.PostPlan]; ok { + if err := b.waitTaskStage(stopCtx, cancelCtx, op, r, stage.ID, "Post-plan Tasks"); err != nil { return r, err } } @@ -382,19 +373,6 @@ in order to capture the filesystem context the remote workspace expects: return r, nil } -func getTaskStageIDByName(stages []*tfe.TaskStage, stageName tfe.Stage) *string { - if len(stages) == 0 { - return nil - } - - for _, stage := range stages { - if stage.Stage == stageName { - return &stage.ID - } - } - return nil -} - const planDefaultHeader = ` [reset][yellow]Running plan in Terraform Cloud. Output will stream here. Pressing Ctrl-C will stop streaming the logs, but will not stop the plan running remotely.[reset] diff --git a/internal/cloud/backend_taskStages.go b/internal/cloud/backend_taskStages.go new file mode 100644 index 000000000000..d2ae881b2754 --- /dev/null +++ b/internal/cloud/backend_taskStages.go @@ -0,0 +1,32 @@ +package cloud + +import ( + "context" + "strings" + + tfe "github.com/hashicorp/go-tfe" +) + +type taskStages map[tfe.Stage]*tfe.TaskStage + +func (b *Cloud) runTaskStages(ctx context.Context, client *tfe.Client, runId string) (taskStages, error) { + taskStages := make(taskStages, 0) + result, err := client.Runs.ReadWithOptions(ctx, runId, &tfe.RunReadOptions{ + Include: []tfe.RunIncludeOpt{tfe.RunTaskStages}, + }) + if err == nil { + for _, t := range result.TaskStages { + if t != nil { + taskStages[t.Stage] = t + } + } + } else { + // This error would be expected for older versions of TFE that do not allow + // fetching task_stages. + if !strings.HasSuffix(err.Error(), "Invalid include parameter") { + return taskStages, generalError("Failed to retrieve run", err) + } + } + + return taskStages, nil +} From 43371b829368a21601787def7143614ad2851d4f Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Tue, 20 Sep 2022 16:08:23 +0800 Subject: [PATCH 3/4] Add support for pre-apply task results in the cloud backend Previously the cloud backend only supported the pre and post plan stages. This commit adds support to display the output of the new pre-apply task stage as well. --- internal/cloud/backend_apply.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/internal/cloud/backend_apply.go b/internal/cloud/backend_apply.go index ff22dd5a9091..078045540ecd 100644 --- a/internal/cloud/backend_apply.go +++ b/internal/cloud/backend_apply.go @@ -133,6 +133,19 @@ func (b *Cloud) opApply(stopCtx, cancelCtx context.Context, op *backend.Operatio } } + // Retrieve the run to get task stages. + // Task Stages are calculated upfront so we only need to call this once for the run. + taskStages, err := b.runTaskStages(stopCtx, b.client, r.ID) + if err != nil { + return r, err + } + + if stage, ok := taskStages[tfe.PreApply]; ok { + if err := b.waitTaskStage(stopCtx, cancelCtx, op, r, stage.ID, "Pre-apply Tasks"); err != nil { + return r, err + } + } + r, err = b.waitForRun(stopCtx, cancelCtx, op, "apply", r, w) if err != nil { return r, err From 1773129832894a90bbc90f93759b8494981baced Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Wed, 21 Sep 2022 09:40:52 +0800 Subject: [PATCH 4/4] Add tests for cloud backend taskStage --- internal/cloud/backend_taskStages_test.go | 207 ++++++++++++++++++++++ 1 file changed, 207 insertions(+) create mode 100644 internal/cloud/backend_taskStages_test.go diff --git a/internal/cloud/backend_taskStages_test.go b/internal/cloud/backend_taskStages_test.go new file mode 100644 index 000000000000..e52f6a5e701e --- /dev/null +++ b/internal/cloud/backend_taskStages_test.go @@ -0,0 +1,207 @@ +package cloud + +import ( + "context" + "errors" + "testing" + + "github.com/golang/mock/gomock" + "github.com/hashicorp/go-tfe" + tfemocks "github.com/hashicorp/go-tfe/mocks" +) + +func MockAllTaskStages(t *testing.T, client *tfe.Client) (RunID string) { + ctrl := gomock.NewController(t) + + RunID = "run-all_task_stages" + + mockRunsAPI := tfemocks.NewMockRuns(ctrl) + + goodRun := tfe.Run{ + TaskStages: []*tfe.TaskStage{ + { + Stage: tfe.PrePlan, + }, + { + Stage: tfe.PostPlan, + }, + { + Stage: tfe.PreApply, + }, + }, + } + mockRunsAPI. + EXPECT(). + ReadWithOptions(gomock.Any(), RunID, gomock.Any()). + Return(&goodRun, nil). + AnyTimes() + + // Mock a bad Read response + mockRunsAPI. + EXPECT(). + ReadWithOptions(gomock.Any(), gomock.Any(), gomock.Any()). + Return(nil, tfe.ErrInvalidOrg). + AnyTimes() + + // Wire up the mock interfaces + client.Runs = mockRunsAPI + return +} + +func MockPrePlanTaskStage(t *testing.T, client *tfe.Client) (RunID string) { + ctrl := gomock.NewController(t) + + RunID = "run-pre_plan_task_stage" + + mockRunsAPI := tfemocks.NewMockRuns(ctrl) + + goodRun := tfe.Run{ + TaskStages: []*tfe.TaskStage{ + { + Stage: tfe.PrePlan, + }, + }, + } + mockRunsAPI. + EXPECT(). + ReadWithOptions(gomock.Any(), RunID, gomock.Any()). + Return(&goodRun, nil). + AnyTimes() + + // Mock a bad Read response + mockRunsAPI. + EXPECT(). + ReadWithOptions(gomock.Any(), gomock.Any(), gomock.Any()). + Return(nil, tfe.ErrInvalidOrg). + AnyTimes() + + // Wire up the mock interfaces + client.Runs = mockRunsAPI + return +} + +func MockTaskStageUnsupported(t *testing.T, client *tfe.Client) (RunID string) { + ctrl := gomock.NewController(t) + + RunID = "run-unsupported_task_stage" + + mockRunsAPI := tfemocks.NewMockRuns(ctrl) + + mockRunsAPI. + EXPECT(). + ReadWithOptions(gomock.Any(), RunID, gomock.Any()). + Return(nil, errors.New("Invalid include parameter")). + AnyTimes() + + mockRunsAPI. + EXPECT(). + ReadWithOptions(gomock.Any(), gomock.Any(), gomock.Any()). + Return(nil, tfe.ErrInvalidOrg). + AnyTimes() + + client.Runs = mockRunsAPI + return +} + +func TestTaskStagesWithAllStages(t *testing.T) { + b, bCleanup := testBackendWithName(t) + defer bCleanup() + + config := &tfe.Config{ + Token: "not-a-token", + } + client, _ := tfe.NewClient(config) + runID := MockAllTaskStages(t, client) + + ctx := context.TODO() + taskStages, err := b.runTaskStages(ctx, client, runID) + + if err != nil { + t.Fatalf("Expected to not error but received %s", err) + } + + for _, stageName := range []tfe.Stage{ + tfe.PrePlan, + tfe.PostPlan, + tfe.PreApply, + } { + if stage, ok := taskStages[stageName]; ok { + if stage.Stage != stageName { + t.Errorf("Expected task stage indexed by %s to find a Task Stage with the same index, but receieved %s", stageName, stage.Stage) + } + } else { + t.Errorf("Expected task stage indexed by %s to exist, but it did not", stageName) + } + } +} + +func TestTaskStagesWithOneStage(t *testing.T) { + b, bCleanup := testBackendWithName(t) + defer bCleanup() + + config := &tfe.Config{ + Token: "not-a-token", + } + client, _ := tfe.NewClient(config) + runID := MockPrePlanTaskStage(t, client) + + ctx := context.TODO() + taskStages, err := b.runTaskStages(ctx, client, runID) + + if err != nil { + t.Fatalf("Expected to not error but received %s", err) + } + + if _, ok := taskStages[tfe.PrePlan]; !ok { + t.Errorf("Expected task stage indexed by %s to exist, but it did not", tfe.PrePlan) + } + + for _, stageName := range []tfe.Stage{ + tfe.PostPlan, + tfe.PreApply, + } { + if _, ok := taskStages[stageName]; ok { + t.Errorf("Expected task stage indexed by %s to not exist, but it did", stageName) + } + } +} + +func TestTaskStagesWithOldTFC(t *testing.T) { + b, bCleanup := testBackendWithName(t) + defer bCleanup() + + config := &tfe.Config{ + Token: "not-a-token", + } + client, _ := tfe.NewClient(config) + runID := MockTaskStageUnsupported(t, client) + + ctx := context.TODO() + taskStages, err := b.runTaskStages(ctx, client, runID) + + if err != nil { + t.Fatalf("Expected to not error but received %s", err) + } + + if len(taskStages) != 0 { + t.Errorf("Expected task stage to be empty, but found %d stages", len(taskStages)) + } +} + +func TestTaskStagesWithErrors(t *testing.T) { + b, bCleanup := testBackendWithName(t) + defer bCleanup() + + config := &tfe.Config{ + Token: "not-a-token", + } + client, _ := tfe.NewClient(config) + MockTaskStageUnsupported(t, client) + + ctx := context.TODO() + _, err := b.runTaskStages(ctx, client, "this run ID will not exist is invalid anyway") + + if err == nil { + t.Error("Expected to error but did not") + } +}