From 8635cb1cfb78d134df93ebf48983cbf5748f51be Mon Sep 17 00:00:00 2001 From: Bruno Schaatsbergen Date: Wed, 30 Oct 2024 19:50:33 +0100 Subject: [PATCH 01/11] views: move retryLog hook to init view The cloud backend includes a retry hook that relies on a go-tfe implementation. Since these retry messages were not rendered through a view, the `-json` flag was not respected, leading to mixed (non-pure JSON) output in the terminal whenever retry messages were printed. Before passing the init view to the cloud backend, we ensure the retryLog hook is included in the init view. Signed-off-by: Bruno Schaatsbergen --- .../command/views/{cloud.go => hook_cloud.go} | 24 ++++++++++++------- internal/command/views/init.go | 14 +++++++++++ 2 files changed, 30 insertions(+), 8 deletions(-) rename internal/command/views/{cloud.go => hook_cloud.go} (78%) diff --git a/internal/command/views/cloud.go b/internal/command/views/hook_cloud.go similarity index 78% rename from internal/command/views/cloud.go rename to internal/command/views/hook_cloud.go index 4842360507c8..870111368ec4 100644 --- a/internal/command/views/cloud.go +++ b/internal/command/views/hook_cloud.go @@ -1,6 +1,3 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: BUSL-1.1 - package views import ( @@ -8,21 +5,32 @@ import ( "net/http" "strings" "time" + + "github.com/hashicorp/terraform/internal/terraform" ) -// CloudHooks provides functions that help with integrating directly into -// the go-tfe tfe.Client struct. -type CloudHooks struct { - // lastRetry is set to the last time a request was retried. +type RetryLogHook struct { + terraform.NilHook + + view *View + lastRetry time.Time } +var _ terraform.Hook = (*UiHook)(nil) + +func NewRetryLoghook(view *View) *RetryLogHook { + return &RetryLogHook{ + view: view, + } +} + // RetryLogHook returns a string providing an update about a request from the // client being retried. // // If colorize is true, then the value returned by this function should be // processed by a colorizer. -func (hooks *CloudHooks) RetryLogHook(attemptNum int, resp *http.Response, colorize bool) string { +func (hooks *RetryLogHook) RetryLogHook(attemptNum int, resp *http.Response, colorize bool) string { // Ignore the first retry to make sure any delayed output will // be written to the console before we start logging retries. // diff --git a/internal/command/views/init.go b/internal/command/views/init.go index 92868d11615f..e21d1403eb75 100644 --- a/internal/command/views/init.go +++ b/internal/command/views/init.go @@ -10,12 +10,14 @@ import ( "time" "github.com/hashicorp/terraform/internal/command/arguments" + "github.com/hashicorp/terraform/internal/terraform" "github.com/hashicorp/terraform/internal/tfdiags" ) // The Init view is used for the init command. type Init interface { Diagnostics(diags tfdiags.Diagnostics) + Hooks() []terraform.Hook Output(messageCode InitMessageCode, params ...any) LogInitMessage(messageCode InitMessageCode, params ...any) Log(message string, params ...any) @@ -50,6 +52,12 @@ func (v *InitHuman) Diagnostics(diags tfdiags.Diagnostics) { v.view.Diagnostics(diags) } +func (v *InitHuman) Hooks() []terraform.Hook { + return []terraform.Hook{ + NewRetryLoghook(v.view), + } +} + func (v *InitHuman) Output(messageCode InitMessageCode, params ...any) { v.view.streams.Println(v.PrepareMessage(messageCode, params...)) } @@ -90,6 +98,12 @@ func (v *InitJSON) Diagnostics(diags tfdiags.Diagnostics) { v.view.Diagnostics(diags) } +func (v *InitJSON) Hooks() []terraform.Hook { + return []terraform.Hook{ + NewRetryLoghook(v.view.view), + } +} + func (v *InitJSON) Output(messageCode InitMessageCode, params ...any) { // don't add empty messages to json output preppedMessage := v.PrepareMessage(messageCode, params...) From f5dc158709899fce290a039155250177f3d61a6e Mon Sep 17 00:00:00 2001 From: Bruno Schaatsbergen Date: Wed, 30 Oct 2024 19:57:11 +0100 Subject: [PATCH 02/11] views: render retry messages using the view Signed-off-by: Bruno Schaatsbergen --- internal/command/views/hook_cloud.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/command/views/hook_cloud.go b/internal/command/views/hook_cloud.go index 870111368ec4..a9130d29d91b 100644 --- a/internal/command/views/hook_cloud.go +++ b/internal/command/views/hook_cloud.go @@ -52,7 +52,7 @@ func (hooks *RetryLogHook) RetryLogHook(attemptNum int, resp *http.Response, col if colorize { return strings.TrimSpace(fmt.Sprintf("[reset][yellow]%s[reset]", msg)) } - return strings.TrimSpace(msg) + return hooks.view.colorize.Color(strings.TrimSpace(msg)) } // The newline in this error is to make it look good in the CLI! From f9797595e36744dcd12c70de09050f7b3016c9b2 Mon Sep 17 00:00:00 2001 From: Bruno Schaatsbergen Date: Wed, 30 Oct 2024 22:51:15 +0100 Subject: [PATCH 03/11] chore: initialize new view in meta backend Signed-off-by: Bruno Schaatsbergen --- internal/backend/backendrun/cli.go | 4 ++++ internal/cloud/backend.go | 8 ++++---- internal/cloud/backend_cli.go | 2 ++ internal/command/meta_backend.go | 8 +++++--- .../views/{hook_cloud.go => hook_retry_log.go} | 11 +++++++---- internal/command/views/test.go | 8 ++++---- 6 files changed, 26 insertions(+), 15 deletions(-) rename internal/command/views/{hook_cloud.go => hook_retry_log.go} (80%) diff --git a/internal/backend/backendrun/cli.go b/internal/backend/backendrun/cli.go index 01fd90b0d004..eeae51ae28db 100644 --- a/internal/backend/backendrun/cli.go +++ b/internal/backend/backendrun/cli.go @@ -8,6 +8,7 @@ import ( "github.com/mitchellh/colorstring" "github.com/hashicorp/terraform/internal/backend" + "github.com/hashicorp/terraform/internal/command/views" "github.com/hashicorp/terraform/internal/terminal" "github.com/hashicorp/terraform/internal/terraform" ) @@ -60,6 +61,9 @@ type CLIOpts struct { // for tailoring the output to fit the attached terminal, for example. Streams *terminal.Streams + //FIXME: something + View *views.View + // StatePath is the local path where state is read from. // // StateOutPath is the local path where the state will be written. diff --git a/internal/cloud/backend.go b/internal/cloud/backend.go index 9d20de77553e..f9ea25f3956b 100644 --- a/internal/cloud/backend.go +++ b/internal/cloud/backend.go @@ -65,9 +65,8 @@ type Cloud struct { // client is the HCP Terraform or Terraform Enterprise API client. client *tfe.Client - // viewHooks implements functions integrating the tfe.Client with the CLI - // output. - viewHooks views.CloudHooks + //FIXME: something + View *views.View // Hostname of HCP Terraform or Terraform Enterprise Hostname string @@ -607,7 +606,8 @@ func cliConfigToken(hostname svchost.Hostname, services *disco.Disco) (string, e // backend to log any connection issues to prevent data loss. func (b *Cloud) retryLogHook(attemptNum int, resp *http.Response) { if b.CLI != nil { - if output := b.viewHooks.RetryLogHook(attemptNum, resp, true); len(output) > 0 { + retryLogHook := views.NewRetryLoghook(b.View) + if output := retryLogHook.RetryLogHook(attemptNum, resp, true); len(output) > 0 { b.CLI.Output(b.Colorize().Color(output)) } } diff --git a/internal/cloud/backend_cli.go b/internal/cloud/backend_cli.go index c63746ec037d..5d5bf1f98c41 100644 --- a/internal/cloud/backend_cli.go +++ b/internal/cloud/backend_cli.go @@ -25,6 +25,8 @@ func (b *Cloud) CLIInit(opts *backendrun.CLIOpts) error { Streams: opts.Streams, Colorize: opts.CLIColor, } + //FIXME: something + b.View = opts.View return nil } diff --git a/internal/command/meta_backend.go b/internal/command/meta_backend.go index de9c8772820c..9337b754eb1a 100644 --- a/internal/command/meta_backend.go +++ b/internal/command/meta_backend.go @@ -386,9 +386,11 @@ func (m *Meta) backendCLIOpts() (*backendrun.CLIOpts, error) { return nil, err } return &backendrun.CLIOpts{ - CLI: m.Ui, - CLIColor: m.Colorize(), - Streams: m.Streams, + CLI: m.Ui, + CLIColor: m.Colorize(), + Streams: m.Streams, + //FIXME: something + View: views.NewView(m.Streams), StatePath: m.statePath, StateOutPath: m.stateOutPath, StateBackupPath: m.backupPath, diff --git a/internal/command/views/hook_cloud.go b/internal/command/views/hook_retry_log.go similarity index 80% rename from internal/command/views/hook_cloud.go rename to internal/command/views/hook_retry_log.go index a9130d29d91b..fa1df2fcc409 100644 --- a/internal/command/views/hook_cloud.go +++ b/internal/command/views/hook_retry_log.go @@ -1,3 +1,6 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + package views import ( @@ -30,7 +33,7 @@ func NewRetryLoghook(view *View) *RetryLogHook { // // If colorize is true, then the value returned by this function should be // processed by a colorizer. -func (hooks *RetryLogHook) RetryLogHook(attemptNum int, resp *http.Response, colorize bool) string { +func (hook *RetryLogHook) RetryLogHook(attemptNum int, resp *http.Response, colorize bool) string { // Ignore the first retry to make sure any delayed output will // be written to the console before we start logging retries. // @@ -38,7 +41,7 @@ func (hooks *RetryLogHook) RetryLogHook(attemptNum int, resp *http.Response, col // requests and server errors, but in the cloud backend we only // care about server errors so we ignore rate limit (429) errors. if attemptNum == 0 || (resp != nil && resp.StatusCode == 429) { - hooks.lastRetry = time.Now() + hook.lastRetry = time.Now() return "" } @@ -46,13 +49,13 @@ func (hooks *RetryLogHook) RetryLogHook(attemptNum int, resp *http.Response, col if attemptNum == 1 { msg = initialRetryError } else { - msg = fmt.Sprintf(repeatedRetryError, time.Since(hooks.lastRetry).Round(time.Second)) + msg = fmt.Sprintf(repeatedRetryError, time.Since(hook.lastRetry).Round(time.Second)) } if colorize { return strings.TrimSpace(fmt.Sprintf("[reset][yellow]%s[reset]", msg)) } - return hooks.view.colorize.Color(strings.TrimSpace(msg)) + return hook.view.colorize.Color(strings.TrimSpace(msg)) } // The newline in this error is to make it look good in the CLI! diff --git a/internal/command/views/test.go b/internal/command/views/test.go index 2fbb85a56318..5c363ede381d 100644 --- a/internal/command/views/test.go +++ b/internal/command/views/test.go @@ -101,7 +101,7 @@ func NewTest(vt arguments.ViewType, view *View) Test { } type TestHuman struct { - CloudHooks + RetryLogHook view *View } @@ -371,11 +371,11 @@ func (t *TestHuman) TFCStatusUpdate(status tfe.TestRunStatus, elapsed time.Durat } func (t *TestHuman) TFCRetryHook(attemptNum int, resp *http.Response) { - t.view.streams.Println(t.view.colorize.Color(t.RetryLogHook(attemptNum, resp, true))) + t.view.streams.Println(t.view.colorize.Color(t.RetryLogHook.RetryLogHook(attemptNum, resp, true))) } type TestJSON struct { - CloudHooks + RetryLogHook view *JSONView } @@ -730,7 +730,7 @@ func (t *TestJSON) TFCStatusUpdate(status tfe.TestRunStatus, elapsed time.Durati func (t *TestJSON) TFCRetryHook(attemptNum int, resp *http.Response) { t.view.log.Error( - t.RetryLogHook(attemptNum, resp, false), + t.RetryLogHook.RetryLogHook(attemptNum, resp, false), "type", json.MessageTestRetry, ) } From a90930427a68d446138fec3af6d9b2ad27e55d01 Mon Sep 17 00:00:00 2001 From: Bruno Schaatsbergen Date: Fri, 1 Nov 2024 11:47:09 +0100 Subject: [PATCH 04/11] chore: remove the retrylog hook from the init view Signed-off-by: Bruno Schaatsbergen --- internal/cloud/backend.go | 4 ++-- internal/command/views/hook_retry_log.go | 2 +- internal/command/views/init.go | 14 -------------- 3 files changed, 3 insertions(+), 17 deletions(-) diff --git a/internal/cloud/backend.go b/internal/cloud/backend.go index f9ea25f3956b..8d7f43b85983 100644 --- a/internal/cloud/backend.go +++ b/internal/cloud/backend.go @@ -606,8 +606,8 @@ func cliConfigToken(hostname svchost.Hostname, services *disco.Disco) (string, e // backend to log any connection issues to prevent data loss. func (b *Cloud) retryLogHook(attemptNum int, resp *http.Response) { if b.CLI != nil { - retryLogHook := views.NewRetryLoghook(b.View) - if output := retryLogHook.RetryLogHook(attemptNum, resp, true); len(output) > 0 { + h := views.NewRetryLoghook(b.View) + if output := h.RetryLogHook(attemptNum, resp, true); len(output) > 0 { b.CLI.Output(b.Colorize().Color(output)) } } diff --git a/internal/command/views/hook_retry_log.go b/internal/command/views/hook_retry_log.go index fa1df2fcc409..b0c59ad2ab1e 100644 --- a/internal/command/views/hook_retry_log.go +++ b/internal/command/views/hook_retry_log.go @@ -55,7 +55,7 @@ func (hook *RetryLogHook) RetryLogHook(attemptNum int, resp *http.Response, colo if colorize { return strings.TrimSpace(fmt.Sprintf("[reset][yellow]%s[reset]", msg)) } - return hook.view.colorize.Color(strings.TrimSpace(msg)) + return strings.TrimSpace(msg) } // The newline in this error is to make it look good in the CLI! diff --git a/internal/command/views/init.go b/internal/command/views/init.go index e21d1403eb75..92868d11615f 100644 --- a/internal/command/views/init.go +++ b/internal/command/views/init.go @@ -10,14 +10,12 @@ import ( "time" "github.com/hashicorp/terraform/internal/command/arguments" - "github.com/hashicorp/terraform/internal/terraform" "github.com/hashicorp/terraform/internal/tfdiags" ) // The Init view is used for the init command. type Init interface { Diagnostics(diags tfdiags.Diagnostics) - Hooks() []terraform.Hook Output(messageCode InitMessageCode, params ...any) LogInitMessage(messageCode InitMessageCode, params ...any) Log(message string, params ...any) @@ -52,12 +50,6 @@ func (v *InitHuman) Diagnostics(diags tfdiags.Diagnostics) { v.view.Diagnostics(diags) } -func (v *InitHuman) Hooks() []terraform.Hook { - return []terraform.Hook{ - NewRetryLoghook(v.view), - } -} - func (v *InitHuman) Output(messageCode InitMessageCode, params ...any) { v.view.streams.Println(v.PrepareMessage(messageCode, params...)) } @@ -98,12 +90,6 @@ func (v *InitJSON) Diagnostics(diags tfdiags.Diagnostics) { v.view.Diagnostics(diags) } -func (v *InitJSON) Hooks() []terraform.Hook { - return []terraform.Hook{ - NewRetryLoghook(v.view.view), - } -} - func (v *InitJSON) Output(messageCode InitMessageCode, params ...any) { // don't add empty messages to json output preppedMessage := v.PrepareMessage(messageCode, params...) From 377c959c33a182f34f31d3692f6f285fb21bce83 Mon Sep 17 00:00:00 2001 From: Bruno Schaatsbergen Date: Fri, 1 Nov 2024 15:40:51 +0100 Subject: [PATCH 05/11] views: add cloud view The cloud view is responsible for rendering messages related to cloud operations, presenting them to the user in either a human-readable format or as JSON output in the terminal. Signed-off-by: Bruno Schaatsbergen --- internal/command/views/cloud.go | 196 +++++++++++++++++++++++ internal/command/views/hook_retry_log.go | 67 -------- 2 files changed, 196 insertions(+), 67 deletions(-) create mode 100644 internal/command/views/cloud.go delete mode 100644 internal/command/views/hook_retry_log.go diff --git a/internal/command/views/cloud.go b/internal/command/views/cloud.go new file mode 100644 index 000000000000..314171eca267 --- /dev/null +++ b/internal/command/views/cloud.go @@ -0,0 +1,196 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package views + +import ( + "encoding/json" + "fmt" + "net/http" + "strings" + "time" + + "github.com/hashicorp/terraform/internal/command/arguments" + "github.com/hashicorp/terraform/internal/tfdiags" +) + +// The Cloud view is used for operations that are specific to cloud operations. +type Cloud interface { + RetryLog(attemptNum int, resp *http.Response) + Diagnostics(diags tfdiags.Diagnostics) + Output(messageCode CloudMessageCode, params ...any) +} + +// NewCloud returns Cloud implementation for the given ViewType. +func NewCloud(vt arguments.ViewType, view *View) Cloud { + switch vt { + case arguments.ViewJSON: + return &CloudJSON{ + view: NewJSONView(view), + } + case arguments.ViewHuman: + return &CloudHuman{ + view: view, + } + default: + panic(fmt.Sprintf("unknown view type %v", vt)) + } +} + +// The CloudHuman implementation renders human-readable text logs, suitable for +// a scrolling terminal. +type CloudHuman struct { + view *View + + lastRetry time.Time +} + +var _ Cloud = (*CloudHuman)(nil) + +func (v *CloudHuman) Diagnostics(diags tfdiags.Diagnostics) { + v.view.Diagnostics(diags) +} + +func (v *CloudHuman) RetryLog(attemptNum int, resp *http.Response) { + // Ignore the first retry to make sure any delayed output will + // be written to the console before we start logging retries. + // + // The retry logic in the TFE client will retry both rate limited + // requests and server errors, but in the cloud backend we only + // care about server errors so we ignore rate limit (429) errors. + if attemptNum == 0 || (resp != nil && resp.StatusCode == 429) { + v.lastRetry = time.Now() + return + } + + var msg string + if attemptNum == 1 { + msg = v.PrepareMessage(InitialRetryErrorMessage) + } else { + msg = v.PrepareMessage(RepeatedRetryErrorMessage, time.Since(v.lastRetry).Round(time.Second)) + } + + v.view.streams.Println(msg) +} + +func (v *CloudHuman) Output(messageCode CloudMessageCode, params ...any) { + v.view.streams.Println(v.PrepareMessage(messageCode, params...)) +} + +func (v *CloudHuman) PrepareMessage(messageCode CloudMessageCode, params ...any) string { + message, ok := CloudMessageRegistry[messageCode] + if !ok { + // display the message code as fallback if not found in the message registry + return string(messageCode) + } + + if message.HumanValue == "" { + // no need to apply colorization if the message is empty + return message.HumanValue + } + + return v.view.colorize.Color(strings.TrimSpace(fmt.Sprintf(message.HumanValue, params...))) +} + +// The CloudJSON implementation renders streaming JSON logs, suitable for +// integrating with other software. +type CloudJSON struct { + view *JSONView + + lastRetry time.Time +} + +var _ Cloud = (*CloudJSON)(nil) + +func (v *CloudJSON) Diagnostics(diags tfdiags.Diagnostics) { + v.view.Diagnostics(diags) +} + +func (v *CloudJSON) RetryLog(attemptNum int, resp *http.Response) { + // Ignore the first retry to make sure any delayed output will + // be written to the console before we start logging retries. + // + // The retry logic in the TFE client will retry both rate limited + // requests and server errors, but in the cloud backend we only + // care about server errors so we ignore rate limit (429) errors. + if attemptNum == 0 || (resp != nil && resp.StatusCode == 429) { + v.lastRetry = time.Now() + return + } + + var msg string + if attemptNum == 1 { + msg = v.PrepareMessage(InitialRetryErrorMessage) + } else { + msg = v.PrepareMessage(RepeatedRetryErrorMessage, time.Since(v.lastRetry).Round(time.Second)) + } + + v.view.view.streams.Println(msg) +} + +func (v *CloudJSON) Output(messageCode CloudMessageCode, params ...any) { + // don't add empty messages to json output + preppedMessage := v.PrepareMessage(messageCode, params...) + if preppedMessage == "" { + return + } + + current_timestamp := time.Now().UTC().Format(time.RFC3339) + json_data := map[string]string{ + "@level": "info", + "@message": preppedMessage, + "@module": "terraform.ui", + "@timestamp": current_timestamp, + "type": "cloud_output", + "message_code": string(messageCode), + } + + cloud_output, _ := json.Marshal(json_data) + v.view.view.streams.Println(string(cloud_output)) +} + +func (v *CloudJSON) PrepareMessage(messageCode CloudMessageCode, params ...any) string { + message, ok := CloudMessageRegistry[messageCode] + if !ok { + // display the message code as fallback if not found in the message registry + return string(messageCode) + } + + return strings.TrimSpace(fmt.Sprintf(message.JSONValue, params...)) +} + +// CloudMessage represents a message string in both json and human decorated text format. +type CloudMessage struct { + HumanValue string + JSONValue string +} + +var CloudMessageRegistry map[CloudMessageCode]CloudMessage = map[CloudMessageCode]CloudMessage{ + "initial_retry_error_message": { + HumanValue: initialRetryError, + JSONValue: initialRetryErrorJSON, + }, + "repeated_retry_error_message": { + HumanValue: repeatdRetryError, + JSONValue: repeatdRetryErrorJSON, + }, +} + +type CloudMessageCode string + +const ( + InitialRetryErrorMessage CloudMessageCode = "initial_retry_error_message" + RepeatedRetryErrorMessage CloudMessageCode = "repeated_retry_error_message" +) + +const initialRetryError = `[reset][yellow] +There was an error connecting to HCP Terraform. Please do not exit +Terraform to prevent data loss! Trying to restore the connection...[reset] +` +const initialRetryErrorJSON = ` +There was an error connecting to HCP Terraform. Please do not exit +Terraform to prevent data loss! Trying to restore the connection... +` + +const repeatdRetryError = `[reset][yellow]Still trying to restore the connection... (%s elapsed)[reset]` +const repeatdRetryErrorJSON = `Still trying to restore the connection... (%s elapsed)` diff --git a/internal/command/views/hook_retry_log.go b/internal/command/views/hook_retry_log.go deleted file mode 100644 index b0c59ad2ab1e..000000000000 --- a/internal/command/views/hook_retry_log.go +++ /dev/null @@ -1,67 +0,0 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: BUSL-1.1 - -package views - -import ( - "fmt" - "net/http" - "strings" - "time" - - "github.com/hashicorp/terraform/internal/terraform" -) - -type RetryLogHook struct { - terraform.NilHook - - view *View - - lastRetry time.Time -} - -var _ terraform.Hook = (*UiHook)(nil) - -func NewRetryLoghook(view *View) *RetryLogHook { - return &RetryLogHook{ - view: view, - } -} - -// RetryLogHook returns a string providing an update about a request from the -// client being retried. -// -// If colorize is true, then the value returned by this function should be -// processed by a colorizer. -func (hook *RetryLogHook) RetryLogHook(attemptNum int, resp *http.Response, colorize bool) string { - // Ignore the first retry to make sure any delayed output will - // be written to the console before we start logging retries. - // - // The retry logic in the TFE client will retry both rate limited - // requests and server errors, but in the cloud backend we only - // care about server errors so we ignore rate limit (429) errors. - if attemptNum == 0 || (resp != nil && resp.StatusCode == 429) { - hook.lastRetry = time.Now() - return "" - } - - var msg string - if attemptNum == 1 { - msg = initialRetryError - } else { - msg = fmt.Sprintf(repeatedRetryError, time.Since(hook.lastRetry).Round(time.Second)) - } - - if colorize { - return strings.TrimSpace(fmt.Sprintf("[reset][yellow]%s[reset]", msg)) - } - return strings.TrimSpace(msg) -} - -// The newline in this error is to make it look good in the CLI! -const initialRetryError = ` -There was an error connecting to HCP Terraform. Please do not exit -Terraform to prevent data loss! Trying to restore the connection... -` - -const repeatedRetryError = "Still trying to restore the connection... (%s elapsed)" From d8bfe7a80b378bf6ebdaaa194a68d217eba3e4d9 Mon Sep 17 00:00:00 2001 From: Bruno Schaatsbergen Date: Fri, 1 Nov 2024 16:18:26 +0100 Subject: [PATCH 06/11] cloud: Initialize a new cloud view in the backend to render messages based on view type. Signed-off-by: Bruno Schaatsbergen --- internal/backend/backendrun/cli.go | 8 +++++--- internal/cloud/backend.go | 8 ++------ internal/cloud/backend_cli.go | 5 +++-- internal/command/meta_backend.go | 13 ++++++++----- 4 files changed, 18 insertions(+), 16 deletions(-) diff --git a/internal/backend/backendrun/cli.go b/internal/backend/backendrun/cli.go index eeae51ae28db..c731ef4f5ed8 100644 --- a/internal/backend/backendrun/cli.go +++ b/internal/backend/backendrun/cli.go @@ -8,7 +8,7 @@ import ( "github.com/mitchellh/colorstring" "github.com/hashicorp/terraform/internal/backend" - "github.com/hashicorp/terraform/internal/command/views" + "github.com/hashicorp/terraform/internal/command/arguments" "github.com/hashicorp/terraform/internal/terminal" "github.com/hashicorp/terraform/internal/terraform" ) @@ -61,8 +61,10 @@ type CLIOpts struct { // for tailoring the output to fit the attached terminal, for example. Streams *terminal.Streams - //FIXME: something - View *views.View + // FIXME: Temporarily exposing ViewType to the backend. + // This is a workaround until the backend is refactored to support + // native View handling. + ViewType arguments.ViewType // StatePath is the local path where state is read from. // diff --git a/internal/cloud/backend.go b/internal/cloud/backend.go index 8d7f43b85983..6595c84ac7d0 100644 --- a/internal/cloud/backend.go +++ b/internal/cloud/backend.go @@ -65,8 +65,7 @@ type Cloud struct { // client is the HCP Terraform or Terraform Enterprise API client. client *tfe.Client - //FIXME: something - View *views.View + View views.Cloud // Hostname of HCP Terraform or Terraform Enterprise Hostname string @@ -606,10 +605,7 @@ func cliConfigToken(hostname svchost.Hostname, services *disco.Disco) (string, e // backend to log any connection issues to prevent data loss. func (b *Cloud) retryLogHook(attemptNum int, resp *http.Response) { if b.CLI != nil { - h := views.NewRetryLoghook(b.View) - if output := h.RetryLogHook(attemptNum, resp, true); len(output) > 0 { - b.CLI.Output(b.Colorize().Color(output)) - } + b.View.RetryLog(attemptNum, resp) } } diff --git a/internal/cloud/backend_cli.go b/internal/cloud/backend_cli.go index 5d5bf1f98c41..60edc2711b4b 100644 --- a/internal/cloud/backend_cli.go +++ b/internal/cloud/backend_cli.go @@ -6,6 +6,7 @@ package cloud import ( "github.com/hashicorp/terraform/internal/backend/backendrun" "github.com/hashicorp/terraform/internal/command/jsonformat" + "github.com/hashicorp/terraform/internal/command/views" ) // CLIInit implements backendrun.CLI @@ -25,8 +26,8 @@ func (b *Cloud) CLIInit(opts *backendrun.CLIOpts) error { Streams: opts.Streams, Colorize: opts.CLIColor, } - //FIXME: something - b.View = opts.View + view := views.NewView(opts.Streams) + b.View = views.NewCloud(opts.ViewType, view) return nil } diff --git a/internal/command/meta_backend.go b/internal/command/meta_backend.go index 9337b754eb1a..65155be5ae9c 100644 --- a/internal/command/meta_backend.go +++ b/internal/command/meta_backend.go @@ -151,6 +151,11 @@ func (m *Meta) Backend(opts *BackendOpts) (backendrun.OperationsBackend, tfdiags } cliOpts.Validation = true + // FIXME: Temporarily exposing ViewType to the backend. + // This is a workaround until the backend is refactored to support + // native View handling. + cliOpts.ViewType = opts.ViewType + // If the backend supports CLI initialization, do it. if cli, ok := b.(backendrun.CLI); ok { if err := cli.CLIInit(cliOpts); err != nil { @@ -386,11 +391,9 @@ func (m *Meta) backendCLIOpts() (*backendrun.CLIOpts, error) { return nil, err } return &backendrun.CLIOpts{ - CLI: m.Ui, - CLIColor: m.Colorize(), - Streams: m.Streams, - //FIXME: something - View: views.NewView(m.Streams), + CLI: m.Ui, + CLIColor: m.Colorize(), + Streams: m.Streams, StatePath: m.statePath, StateOutPath: m.stateOutPath, StateBackupPath: m.backupPath, From 6c14e52d8e7802eae7e94631e3a2d73db6b797fc Mon Sep 17 00:00:00 2001 From: Bruno Schaatsbergen Date: Fri, 1 Nov 2024 16:18:42 +0100 Subject: [PATCH 07/11] test: use Cloud view Signed-off-by: Bruno Schaatsbergen --- internal/command/views/test.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/internal/command/views/test.go b/internal/command/views/test.go index 5c363ede381d..742df1887dd8 100644 --- a/internal/command/views/test.go +++ b/internal/command/views/test.go @@ -101,7 +101,7 @@ func NewTest(vt arguments.ViewType, view *View) Test { } type TestHuman struct { - RetryLogHook + Cloud view *View } @@ -371,11 +371,11 @@ func (t *TestHuman) TFCStatusUpdate(status tfe.TestRunStatus, elapsed time.Durat } func (t *TestHuman) TFCRetryHook(attemptNum int, resp *http.Response) { - t.view.streams.Println(t.view.colorize.Color(t.RetryLogHook.RetryLogHook(attemptNum, resp, true))) + t.Cloud.RetryLog(attemptNum, resp) } type TestJSON struct { - RetryLogHook + Cloud view *JSONView } @@ -729,10 +729,7 @@ func (t *TestJSON) TFCStatusUpdate(status tfe.TestRunStatus, elapsed time.Durati } func (t *TestJSON) TFCRetryHook(attemptNum int, resp *http.Response) { - t.view.log.Error( - t.RetryLogHook.RetryLogHook(attemptNum, resp, false), - "type", json.MessageTestRetry, - ) + t.Cloud.RetryLog(attemptNum, resp) } // TestJUnitXMLFile produces a JUnit XML file at the conclusion of a test From 09ddfc039ff270e43ae80373d335f485fc6c07e3 Mon Sep 17 00:00:00 2001 From: Bruno Schaatsbergen Date: Mon, 4 Nov 2024 16:31:05 +0100 Subject: [PATCH 08/11] docs: add a comment to the View field Signed-off-by: Bruno Schaatsbergen --- internal/cloud/backend.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/cloud/backend.go b/internal/cloud/backend.go index 6595c84ac7d0..b2b862553583 100644 --- a/internal/cloud/backend.go +++ b/internal/cloud/backend.go @@ -65,6 +65,7 @@ type Cloud struct { // client is the HCP Terraform or Terraform Enterprise API client. client *tfe.Client + // View handles rendering output in human-readable or machine-readable format from cloud-specific operations. View views.Cloud // Hostname of HCP Terraform or Terraform Enterprise From 1e80cd8e103e40158d818c796ff8d38206344aef Mon Sep 17 00:00:00 2001 From: Bruno Schaatsbergen Date: Mon, 4 Nov 2024 18:04:08 +0100 Subject: [PATCH 09/11] cloud: clarify reason for guard statement Added a comment to explain the rationale behind the guard statement, which prevents potential nil errors during backend initialization and retryLogHook call. Signed-off-by: Bruno Schaatsbergen --- internal/cloud/backend.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/internal/cloud/backend.go b/internal/cloud/backend.go index b2b862553583..4a017eae66af 100644 --- a/internal/cloud/backend.go +++ b/internal/cloud/backend.go @@ -605,6 +605,13 @@ func cliConfigToken(hostname svchost.Hostname, services *disco.Disco) (string, e // retryLogHook is invoked each time a request is retried allowing the // backend to log any connection issues to prevent data loss. func (b *Cloud) retryLogHook(attemptNum int, resp *http.Response) { + // FIXME: This guard statement prevents a potential nil error + // due to the way the backend is initialized and the context from which + // this function is called. + // + // In a future refactor, we should ensure that views are natively supported + // in backends and allow for calling a View directly within the + // backend.Configure method. if b.CLI != nil { b.View.RetryLog(attemptNum, resp) } From c9f418d85376fef81bb61cecb9679888d2025330 Mon Sep 17 00:00:00 2001 From: Bruno Schaatsbergen Date: Mon, 4 Nov 2024 18:47:21 +0100 Subject: [PATCH 10/11] views: add PrepareMessage to cloud view interface Signed-off-by: Bruno Schaatsbergen --- internal/command/views/cloud.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/command/views/cloud.go b/internal/command/views/cloud.go index 314171eca267..8b9d4adeef73 100644 --- a/internal/command/views/cloud.go +++ b/internal/command/views/cloud.go @@ -19,6 +19,7 @@ type Cloud interface { RetryLog(attemptNum int, resp *http.Response) Diagnostics(diags tfdiags.Diagnostics) Output(messageCode CloudMessageCode, params ...any) + PrepareMessage(messageCode CloudMessageCode, params ...any) string } // NewCloud returns Cloud implementation for the given ViewType. From ad1f3e0adcb3f4d4ae03041ee2561d2a4ea1a441 Mon Sep 17 00:00:00 2001 From: Bruno Schaatsbergen Date: Mon, 4 Nov 2024 18:47:34 +0100 Subject: [PATCH 11/11] views: add tests for cloud view Signed-off-by: Bruno Schaatsbergen --- internal/command/views/cloud_test.go | 262 +++++++++++++++++++++++++++ 1 file changed, 262 insertions(+) create mode 100644 internal/command/views/cloud_test.go diff --git a/internal/command/views/cloud_test.go b/internal/command/views/cloud_test.go new file mode 100644 index 000000000000..358a895bddc4 --- /dev/null +++ b/internal/command/views/cloud_test.go @@ -0,0 +1,262 @@ +package views + +import ( + "fmt" + "strings" + "testing" + "time" + + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/terraform/internal/command/arguments" + "github.com/hashicorp/terraform/internal/terminal" + "github.com/hashicorp/terraform/internal/tfdiags" + tfversion "github.com/hashicorp/terraform/version" +) + +func TestNewCloud_unsupportedViewDiagnostics(t *testing.T) { + defer func() { + r := recover() + if r == nil { + t.Fatalf("should panic with unsupported view type raw") + } else if r != "unknown view type raw" { + t.Fatalf("unexpected panic message: %v", r) + } + }() + + streams, done := terminal.StreamsForTesting(t) + defer done(t) + + NewCloud(arguments.ViewRaw, NewView(streams).SetRunningInAutomation(true)) +} + +func TestNewCloud_humanViewOutput(t *testing.T) { + t.Run("no param", func(t *testing.T) { + streams, done := terminal.StreamsForTesting(t) + + newCloud := NewCloud(arguments.ViewHuman, NewView(streams).SetRunningInAutomation(true)) + if _, ok := newCloud.(*CloudHuman); !ok { + t.Fatalf("unexpected return type %t", newCloud) + } + + newCloud.Output(InitialRetryErrorMessage) + + actual := done(t).All() + expected := "There was an error connecting to HCP Terraform. Please do not exit\nTerraform to prevent data loss! Trying to restore the connection..." + if !strings.Contains(actual, expected) { + t.Fatalf("expected output to contain: %s, but got %s", expected, actual) + } + }) + + t.Run("single param", func(t *testing.T) { + streams, done := terminal.StreamsForTesting(t) + + newCloud := NewCloud(arguments.ViewHuman, NewView(streams).SetRunningInAutomation(true)) + if _, ok := newCloud.(*CloudHuman); !ok { + t.Fatalf("unexpected return type %t", newCloud) + } + + duration := 5 * time.Second + newCloud.Output(RepeatedRetryErrorMessage, duration) + + actual := done(t).All() + expected := fmt.Sprintf("Still trying to restore the connection... (%s elapsed)", duration) + if !strings.Contains(actual, expected) { + t.Fatalf("expected output to contain: %s, but got %s", expected, actual) + } + }) +} + +func TestNewCloud_humanViewPrepareMessage(t *testing.T) { + t.Run("existing message code", func(t *testing.T) { + streams, _ := terminal.StreamsForTesting(t) + + newCloud := NewCloud(arguments.ViewHuman, NewView(streams).SetRunningInAutomation(true)) + if _, ok := newCloud.(*CloudHuman); !ok { + t.Fatalf("unexpected return type %t", newCloud) + } + + want := "\nThere was an error connecting to HCP Terraform. Please do not exit\nTerraform to prevent data loss! Trying to restore the connection..." + + actual := newCloud.PrepareMessage(InitialRetryErrorMessage) + if !cmp.Equal(want, actual) { + t.Errorf("unexpected output: %s", cmp.Diff(want, actual)) + } + }) +} + +func TestNewCloud_humanViewDiagnostics(t *testing.T) { + streams, done := terminal.StreamsForTesting(t) + + newCloud := NewCloud(arguments.ViewHuman, NewView(streams).SetRunningInAutomation(true)) + if _, ok := newCloud.(*CloudHuman); !ok { + t.Fatalf("unexpected return type %t", newCloud) + } + + diags := getHCPDiags(t) + newCloud.Diagnostics(diags) + + actual := done(t).All() + expected := "\nError: Error connecting to HCP\n\nCould not connect to HCP Terraform. Check your network.\n\nError: Network Timeout\n\nConnection to HCP timed out. Check your network.\n" + if !strings.Contains(actual, expected) { + t.Fatalf("expected output to contain: %s, but got %s", expected, actual) + } +} + +func TestNewCloud_jsonViewOutput(t *testing.T) { + t.Run("no param", func(t *testing.T) { + streams, done := terminal.StreamsForTesting(t) + + newCloud := NewCloud(arguments.ViewJSON, NewView(streams).SetRunningInAutomation(true)) + if _, ok := newCloud.(*CloudJSON); !ok { + t.Fatalf("unexpected return type %t", newCloud) + } + + newCloud.Output(InitialRetryErrorMessage) + + version := tfversion.String() + want := []map[string]interface{}{ + { + "@level": "info", + "@message": fmt.Sprintf("Terraform %s", version), + "@module": "terraform.ui", + "terraform": version, + "type": "version", + "ui": JSON_UI_VERSION, + }, + { + "@level": "info", + "@message": "There was an error connecting to HCP Terraform. Please do not exit\nTerraform to prevent data loss! Trying to restore the connection...", + "message_code": "initial_retry_error_message", + "@module": "terraform.ui", + "type": "cloud_output", + }, + } + + actual := done(t).Stdout() + testJSONViewOutputEqualsFull(t, actual, want) + }) + + t.Run("single param", func(t *testing.T) { + streams, done := terminal.StreamsForTesting(t) + + newCloud := NewCloud(arguments.ViewJSON, NewView(streams).SetRunningInAutomation(true)) + if _, ok := newCloud.(*CloudJSON); !ok { + t.Fatalf("unexpected return type %t", newCloud) + } + + duration := 5 * time.Second + newCloud.Output(RepeatedRetryErrorMessage, duration) + + version := tfversion.String() + want := []map[string]interface{}{ + { + "@level": "info", + "@message": fmt.Sprintf("Terraform %s", version), + "@module": "terraform.ui", + "terraform": version, + "type": "version", + "ui": JSON_UI_VERSION, + }, + { + "@level": "info", + "@message": fmt.Sprintf("Still trying to restore the connection... (%s elapsed)", duration), + "@module": "terraform.ui", + "message_code": "repeated_retry_error_message", + "type": "cloud_output", + }, + } + + actual := done(t).Stdout() + testJSONViewOutputEqualsFull(t, actual, want) + }) +} + +func TestNewCloud_jsonViewPrepareMessage(t *testing.T) { + t.Run("existing message code", func(t *testing.T) { + streams, _ := terminal.StreamsForTesting(t) + + newCloud := NewCloud(arguments.ViewJSON, NewView(streams).SetRunningInAutomation(true)) + if _, ok := newCloud.(*CloudJSON); !ok { + t.Fatalf("unexpected return type %t", newCloud) + } + + want := "There was an error connecting to HCP Terraform. Please do not exit\nTerraform to prevent data loss! Trying to restore the connection..." + + actual := newCloud.PrepareMessage(InitialRetryErrorMessage) + if !cmp.Equal(want, actual) { + t.Errorf("unexpected output: %s", cmp.Diff(want, actual)) + } + }) +} + +func TestNewCloud_jsonViewDiagnostics(t *testing.T) { + streams, done := terminal.StreamsForTesting(t) + + newCloud := NewCloud(arguments.ViewJSON, NewView(streams).SetRunningInAutomation(true)) + if _, ok := newCloud.(*CloudJSON); !ok { + t.Fatalf("unexpected return type %t", newCloud) + } + + diags := getHCPDiags(t) // Assuming you want to use the HCP diagnostics here + newCloud.Diagnostics(diags) + + version := tfversion.String() + want := []map[string]interface{}{ + { + "@level": "info", + "@message": fmt.Sprintf("Terraform %s", version), + "@module": "terraform.ui", + "terraform": version, + "type": "version", + "ui": JSON_UI_VERSION, + }, + { + "@level": "error", + "@message": "Error: Error connecting to HCP", + "@module": "terraform.ui", + "diagnostic": map[string]interface{}{ + "severity": "error", + "summary": "Error connecting to HCP", + "detail": "Could not connect to HCP Terraform. Check your network.", + }, + "type": "diagnostic", + }, + { + "@level": "error", + "@message": "Error: Network Timeout", + "@module": "terraform.ui", + "diagnostic": map[string]interface{}{ + "severity": "error", + "summary": "Network Timeout", + "detail": "Connection to HCP timed out. Check your network.", + }, + "type": "diagnostic", + }, + } + + actual := done(t).Stdout() + testJSONViewOutputEqualsFull(t, actual, want) +} + +// These are mock error messages created solely for testing connectivity issues. +func getHCPDiags(t *testing.T) tfdiags.Diagnostics { + t.Helper() + + var diags tfdiags.Diagnostics + diags = diags.Append( + tfdiags.Sourceless( + tfdiags.Error, + "Error connecting to HCP", + "Could not connect to HCP Terraform. Check your network.", + ), + &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Network Timeout", + Detail: "Connection to HCP timed out. Check your network.", + Subject: nil, + }, + ) + + return diags +}