From ddabdde4212160688c52989c506e3b14c1baf9b8 Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Fri, 22 Jul 2022 11:47:26 +0100 Subject: [PATCH 1/2] settings: break down terraform CLI related settings --- docs/SETTINGS.md | 22 ++++++++-- internal/langserver/handlers/initialize.go | 12 +++--- internal/langserver/handlers/service.go | 49 ++++++++++++++++------ internal/settings/settings.go | 23 ++++++---- 4 files changed, 76 insertions(+), 30 deletions(-) diff --git a/docs/SETTINGS.md b/docs/SETTINGS.md index 3595c8ba..fbe18507 100644 --- a/docs/SETTINGS.md +++ b/docs/SETTINGS.md @@ -10,24 +10,40 @@ Clients which expose these config options to the end-user are advised to match t The language server supports the following configuration options: -## `terraformLogFilePath` (`string`) +## `terraform` (object `{}`) + +Terraform CLI related settings (used e.g. in formatting code via `terraform fmt`). + +### `logFilePath` (`string`) Path to a file for Terraform executions to be logged into (`TF_LOG_PATH`) with support for variables (e.g. Timestamp, Pid, Ppid) via Go template syntax `{{.VarName}}` -## `terraformExecTimeout` (`string`) +### `timeout` (`string`) Overrides Terraform execution timeout in [`time.ParseDuration`](https://pkg.go.dev/time#ParseDuration) compatible format (e.g. `30s`) -## `terraformExecPath` (`string`) +### `path` (`string`) Path to the Terraform binary. This is usually looked up automatically from `$PATH` and should not need to be specified in majority of cases. Use this to override the automatic lookup. +## **DEPRECATED**: `terraformLogFilePath` (`string`) + +Deprecated in favour of `terraform.logFilePath` + +## **DEPRECATED**: `terraformExecTimeout` (`string`) + +Deprecated in favour of `terraform.timeout` + +## **DEPRECATED**: `terraformExecPath` (`string`) + +Deprecated in favour of `terraform.path` + ## **DEPRECATED**: `rootModulePaths` (`[]string`) This option is deprecated and ignored from v0.29+, it was used as an escape hatch diff --git a/internal/langserver/handlers/initialize.go b/internal/langserver/handlers/initialize.go index f20c4f2f..d2717300 100644 --- a/internal/langserver/handlers/initialize.go +++ b/internal/langserver/handlers/initialize.go @@ -177,9 +177,9 @@ func getTelemetryProperties(out *settings.DecodedOptions) map[string]interface{} "options.indexing.ignoreDirectoryNames": false, "options.indexing.ignorePaths": false, "options.experimentalFeatures.validateOnSave": false, - "options.terraformExecPath": false, - "options.terraformExecTimeout": "", - "options.terraformLogFilePath": false, + "options.terraform.path": false, + "options.terraform.timeout": "", + "options.terraform.logFilePath": false, "root_uri": "dir", "lsVersion": "", } @@ -192,9 +192,9 @@ func getTelemetryProperties(out *settings.DecodedOptions) map[string]interface{} properties["options.experimentalFeatures.prefillRequiredFields"] = out.Options.ExperimentalFeatures.PrefillRequiredFields properties["options.experimentalFeatures.validateOnSave"] = out.Options.ExperimentalFeatures.ValidateOnSave properties["options.ignoreSingleFileWarning"] = out.Options.IgnoreSingleFileWarning - properties["options.terraformExecPath"] = len(out.Options.TerraformExecPath) > 0 - properties["options.terraformExecTimeout"] = out.Options.TerraformExecTimeout - properties["options.terraformLogFilePath"] = len(out.Options.TerraformLogFilePath) > 0 + properties["options.terraform.path"] = len(out.Options.Terraform.Path) > 0 + properties["options.terraform.timeout"] = out.Options.Terraform.Timeout + properties["options.terraform.logFilePath"] = len(out.Options.Terraform.LogFilePath) > 0 return properties } diff --git a/internal/langserver/handlers/service.go b/internal/langserver/handlers/service.go index d0dccdd8..4804b919 100644 --- a/internal/langserver/handlers/service.go +++ b/internal/langserver/handlers/service.go @@ -372,17 +372,40 @@ func (svc *service) Assigner() (jrpc2.Assigner, error) { } func (svc *service) configureSessionDependencies(ctx context.Context, cfgOpts *settings.Options) error { + // Raise warnings for deprecated options + if cfgOpts.XLegacyTerraformExecPath != "" { + jrpc2.ServerFromContext(ctx).Notify(ctx, "window/showMessage", &lsp.ShowMessageParams{ + Type: lsp.Warning, + Message: fmt.Sprintf("terraformExecPath (%q) is deprecated (no-op), use terraform.path instead", + cfgOpts.XLegacyExcludeModulePaths), + }) + } + if cfgOpts.XLegacyTerraformExecTimeout != "" { + jrpc2.ServerFromContext(ctx).Notify(ctx, "window/showMessage", &lsp.ShowMessageParams{ + Type: lsp.Warning, + Message: fmt.Sprintf("terraformExecTimeout (%q) is deprecated (no-op), use terraform.timeout instead", + cfgOpts.XLegacyExcludeModulePaths), + }) + } + if cfgOpts.XLegacyTerraformExecLogFilePath != "" { + jrpc2.ServerFromContext(ctx).Notify(ctx, "window/showMessage", &lsp.ShowMessageParams{ + Type: lsp.Warning, + Message: fmt.Sprintf("terraformExecLogFilePath (%q) is deprecated (no-op), use terraform.logFilePath instead", + cfgOpts.XLegacyExcludeModulePaths), + }) + } + // The following is set via CLI flags, hence available in the server context execOpts := &exec.ExecutorOpts{} cliExecPath, ok := lsctx.TerraformExecPath(svc.srvCtx) if ok { - if len(cfgOpts.TerraformExecPath) > 0 { + if len(cfgOpts.Terraform.Path) > 0 { return fmt.Errorf("Terraform exec path can either be set via (-tf-exec) CLI flag " + - "or (terraformExecPath) LSP config option, not both") + "or (terraform.path) LSP config option, not both") } execOpts.ExecPath = cliExecPath - } else if len(cfgOpts.TerraformExecPath) > 0 { - execOpts.ExecPath = cfgOpts.TerraformExecPath + } else if len(cfgOpts.Terraform.Path) > 0 { + execOpts.ExecPath = cfgOpts.Terraform.Path } else { path, err := svc.tfDiscoFunc() if err == nil { @@ -393,26 +416,26 @@ func (svc *service) configureSessionDependencies(ctx context.Context, cfgOpts *s path, ok := lsctx.TerraformExecLogPath(svc.srvCtx) if ok { - if len(cfgOpts.TerraformLogFilePath) > 0 { + if len(cfgOpts.Terraform.LogFilePath) > 0 { return fmt.Errorf("Terraform log file path can either be set via (-tf-log-file) CLI flag " + - "or (terraformLogFilePath) LSP config option, not both") + "or (terraform.logFilePath) LSP config option, not both") } execOpts.ExecLogPath = path - } else if len(cfgOpts.TerraformLogFilePath) > 0 { - execOpts.ExecLogPath = cfgOpts.TerraformLogFilePath + } else if len(cfgOpts.Terraform.LogFilePath) > 0 { + execOpts.ExecLogPath = cfgOpts.Terraform.LogFilePath } timeout, ok := lsctx.TerraformExecTimeout(svc.srvCtx) if ok { - if len(cfgOpts.TerraformExecTimeout) > 0 { + if len(cfgOpts.Terraform.Timeout) > 0 { return fmt.Errorf("Terraform exec timeout can either be set via (-tf-exec-timeout) CLI flag " + - "or (terraformExecTimeout) LSP config option, not both") + "or (terraform.timeout) LSP config option, not both") } execOpts.Timeout = timeout - } else if len(cfgOpts.TerraformExecTimeout) > 0 { - d, err := time.ParseDuration(cfgOpts.TerraformExecTimeout) + } else if len(cfgOpts.Terraform.Timeout) > 0 { + d, err := time.ParseDuration(cfgOpts.Terraform.Timeout) if err != nil { - return fmt.Errorf("Failed to parse terraformExecTimeout LSP config option: %s", err) + return fmt.Errorf("Failed to parse terraform.timeout LSP config option: %s", err) } execOpts.Timeout = d } diff --git a/internal/settings/settings.go b/internal/settings/settings.go index d115e6b0..cf65d3fa 100644 --- a/internal/settings/settings.go +++ b/internal/settings/settings.go @@ -20,6 +20,12 @@ type Indexing struct { IgnorePaths []string `mapstructure:"ignorePaths"` } +type Terraform struct { + Path string `mapstructure:"path"` + Timeout string `mapstructure:"timeout"` + LogFilePath string `mapstructure:"logFilePath"` +} + type Options struct { CommandPrefix string `mapstructure:"commandPrefix"` Indexing Indexing `mapstructure:"indexing"` @@ -29,18 +35,19 @@ type Options struct { IgnoreSingleFileWarning bool `mapstructure:"ignoreSingleFileWarning"` - TerraformExecPath string `mapstructure:"terraformExecPath"` - TerraformExecTimeout string `mapstructure:"terraformExecTimeout"` - TerraformLogFilePath string `mapstructure:"terraformLogFilePath"` + Terraform Terraform `mapstructure:"terraform"` - XLegacyModulePaths []string `mapstructure:"rootModulePaths"` - XLegacyExcludeModulePaths []string `mapstructure:"excludeModulePaths"` - XLegacyIgnoreDirectoryNames []string `mapstructure:"ignoreDirectoryNames"` + XLegacyModulePaths []string `mapstructure:"rootModulePaths"` + XLegacyExcludeModulePaths []string `mapstructure:"excludeModulePaths"` + XLegacyIgnoreDirectoryNames []string `mapstructure:"ignoreDirectoryNames"` + XLegacyTerraformExecPath string `mapstructure:"terraformExecPath"` + XLegacyTerraformExecTimeout string `mapstructure:"terraformExecTimeout"` + XLegacyTerraformExecLogFilePath string `mapstructure:"terraformExecLogFilePath"` } func (o *Options) Validate() error { - if o.TerraformExecPath != "" { - path := o.TerraformExecPath + if o.Terraform.Path != "" { + path := o.Terraform.Path if !filepath.IsAbs(path) { return fmt.Errorf("Expected absolute path for Terraform binary, got %q", path) } From 82e8db17df1b65c4a798622f4be24d4fd6771b2f Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Tue, 26 Jul 2022 18:52:29 +0100 Subject: [PATCH 2/2] settings: add test for path validation --- internal/settings/settings_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/internal/settings/settings_test.go b/internal/settings/settings_test.go index dd727b05..f00525df 100644 --- a/internal/settings/settings_test.go +++ b/internal/settings/settings_test.go @@ -88,3 +88,19 @@ func TestValidate_IgnoreDirectoryNames_success(t *testing.T) { t.Fatalf("did not expect error: %s", result) } } + +func TestValidate_relativePath(t *testing.T) { + out, err := DecodeOptions(map[string]interface{}{ + "terraform": map[string]interface{}{ + "path": "relative/path", + }, + }) + if err != nil { + t.Fatal(err) + } + + result := out.Options.Validate() + if result == nil { + t.Fatal("expected decoding of relative path to result in error") + } +}