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
helper/resource: Ensure Terraform CLI logs are written to TF_LOG_PATH_MASK
environment variable value when both TF_ACC_LOG_PATH
and TF_LOG_PATH_MASK
are set
#938
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
```release-note:bug | ||
helper/resource: Ensured Terraform CLI logs are written to `TF_LOG_PATH_MASK` environment variable value when both `TF_ACC_LOG_PATH` and `TF_LOG_PATH_MASK` are set | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,9 +2,11 @@ package plugintest | |
|
||
import ( | ||
"context" | ||
"errors" | ||
"fmt" | ||
"io/ioutil" | ||
"os" | ||
"strings" | ||
|
||
"github.com/hashicorp/terraform-exec/tfexec" | ||
"github.com/hashicorp/terraform-plugin-sdk/v2/internal/logging" | ||
|
@@ -102,7 +104,7 @@ func (h *Helper) Close() error { | |
// If the working directory object is not itself closed by the time the test | ||
// program exits, the Close method on the helper itself will attempt to | ||
// delete it. | ||
func (h *Helper) NewWorkingDir(ctx context.Context) (*WorkingDir, error) { | ||
func (h *Helper) NewWorkingDir(ctx context.Context, t TestControl) (*WorkingDir, error) { | ||
dir, err := ioutil.TempDir(h.baseDir, "work") | ||
if err != nil { | ||
return nil, err | ||
|
@@ -124,6 +126,50 @@ func (h *Helper) NewWorkingDir(ctx context.Context) (*WorkingDir, error) { | |
return nil, fmt.Errorf("unable to create terraform-exec instance: %w", err) | ||
} | ||
|
||
err = tf.SetDisablePluginTLS(true) | ||
|
||
if err != nil { | ||
return nil, fmt.Errorf("unable to disable terraform-exec plugin TLS: %w", err) | ||
} | ||
|
||
err = tf.SetSkipProviderVerify(true) // Only required for Terraform CLI 0.12.x | ||
|
||
var mismatch *tfexec.ErrVersionMismatch | ||
if err != nil && !errors.As(err, &mismatch) { | ||
return nil, fmt.Errorf("unable to disable terraform-exec provider verification: %w", err) | ||
} | ||
|
||
if logPath := os.Getenv(EnvTfAccLogPath); logPath != "" { | ||
logging.HelperResourceTrace( | ||
ctx, | ||
fmt.Sprintf("Setting terraform-exec log path via %s environment variable", EnvTfAccLogPath), | ||
map[string]interface{}{logging.KeyTestTerraformLogPath: logPath}, | ||
) | ||
|
||
if err := tf.SetLogPath(logPath); err != nil { | ||
return nil, fmt.Errorf("unable to set terraform-exec log path (%s): %w", logPath, err) | ||
} | ||
} | ||
|
||
// Similar to helper/logging.LogOutput() and | ||
// terraform-plugin-log/tfsdklog.RegisterTestSink(), the TF_LOG_PATH_MASK | ||
// environment variable should take precedence over TF_ACC_LOG_PATH. | ||
if logPathMask := os.Getenv(EnvTfLogPathMask); logPathMask != "" { | ||
// Escape special characters which may appear if we have subtests | ||
testName := strings.Replace(t.Name(), "/", "__", -1) | ||
logPath := fmt.Sprintf(logPathMask, testName) | ||
|
||
logging.HelperResourceTrace( | ||
ctx, | ||
fmt.Sprintf("Setting terraform-exec log path via %s environment variable", EnvTfLogPathMask), | ||
map[string]interface{}{logging.KeyTestTerraformLogPath: logPath}, | ||
) | ||
|
||
if err := tf.SetLogPath(logPath); err != nil { | ||
return nil, fmt.Errorf("unable to set terraform-exec log path (%s): %w", logPath, err) | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think technically this is spot on, but there is an issue I can imagine could confuse users: if one sets both Could we instead check for those env vars presence, before we commit with a log and the remaining logic that sets There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated the implementation -- does this now do what you had in mind? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👨🍳 💋 |
||
|
||
return &WorkingDir{ | ||
h: h, | ||
tf: tf, | ||
|
@@ -138,7 +184,7 @@ func (h *Helper) NewWorkingDir(ctx context.Context) (*WorkingDir, error) { | |
func (h *Helper) RequireNewWorkingDir(ctx context.Context, t TestControl) *WorkingDir { | ||
t.Helper() | ||
|
||
wd, err := h.NewWorkingDir(ctx) | ||
wd, err := h.NewWorkingDir(ctx, t) | ||
if err != nil { | ||
t := testingT{t} | ||
t.Fatalf("failed to create new working directory: %s", err) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked at https://pkg.go.dev/github.com/hashicorp/terraform-exec@v0.16.1/tfexec but I can't find exactly what setting that
TF_DISABLE_PLUGIN_TLS
means.Does it mean that we want TF to not fetch providers using
https
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should help clarify it slightly:
https://github.com/hashicorp/terraform/blob/01b22f4b7688c9cf084214ef1dfe51ed7719fe42/internal/command/meta_providers.go#L26-L31