diff --git a/cmd/inspect.go b/cmd/inspect.go index 1fa1a298c..b070413bd 100644 --- a/cmd/inspect.go +++ b/cmd/inspect.go @@ -211,7 +211,7 @@ By setting TFLINT_LOG=trace, you can confirm the changes made by the autofix and } func (cli *CLI) setupRunners(opts Options, dir string) ([]*tflint.Runner, error) { - configs, diags := cli.loader.LoadConfig(dir, cli.config.Module) + configs, diags := cli.loader.LoadConfig(dir, cli.config.CallModuleType) if diags.HasErrors() { return []*tflint.Runner{}, fmt.Errorf("Failed to load configurations; %w", diags) } diff --git a/cmd/option.go b/cmd/option.go index 620eb92ef..570463aed 100644 --- a/cmd/option.go +++ b/cmd/option.go @@ -4,6 +4,7 @@ import ( "log" "strings" + "github.com/terraform-linters/tflint/terraform" "github.com/terraform-linters/tflint/tflint" ) @@ -21,8 +22,9 @@ type Options struct { EnablePlugins []string `long:"enable-plugin" description:"Enable plugins from the command line" value-name:"PLUGIN_NAME"` Varfiles []string `long:"var-file" description:"Terraform variable file name" value-name:"FILE"` Variables []string `long:"var" description:"Set a Terraform variable" value-name:"'foo=bar'"` - Module *bool `long:"module" description:"Enable module inspection"` - NoModule *bool `long:"no-module" description:"Disable module inspection"` + Module *bool `long:"module" description:"Enable module inspection" hidden:"true"` + NoModule *bool `long:"no-module" description:"Disable module inspection" hidden:"true"` + CallModuleType *string `long:"call-module-type" description:"Types of module to call (default: local)" choice:"all" choice:"local" choice:"none"` Chdir string `long:"chdir" description:"Switch to a different working directory before executing the command" value-name:"DIR"` Recursive bool `long:"recursive" description:"Run command in each directory recursively"` Filter []string `long:"filter" description:"Filter issues by file names or globs" value-name:"FILE"` @@ -62,6 +64,13 @@ func (opts *Options) toConfig() *tflint.Config { moduleSet = true } + callModuleType := terraform.CallLocalModule + callModuleTypeSet := false + if opts.CallModuleType != nil { + callModuleType = terraform.AsCallModuleType(*opts.CallModuleType) + callModuleTypeSet = true + } + var force, forceSet bool if opts.Force != nil { force = *opts.Force @@ -70,6 +79,7 @@ func (opts *Options) toConfig() *tflint.Config { log.Printf("[DEBUG] CLI Options") log.Printf("[DEBUG] Module: %t", module) + log.Printf("[DEBUG] CallModuleType: %s", callModuleType) log.Printf("[DEBUG] Force: %t", force) log.Printf("[DEBUG] Format: %s", opts.Format) log.Printf("[DEBUG] Varfiles: %s", strings.Join(opts.Varfiles, ", ")) @@ -113,8 +123,10 @@ func (opts *Options) toConfig() *tflint.Config { } return &tflint.Config{ - Module: module, - ModuleSet: moduleSet, + Module: module, + ModuleSet: moduleSet, + CallModuleType: callModuleType, + CallModuleTypeSet: callModuleTypeSet, Force: force, ForceSet: forceSet, diff --git a/langserver/handler.go b/langserver/handler.go index 89159bdf7..0ff81e31b 100644 --- a/langserver/handler.go +++ b/langserver/handler.go @@ -168,7 +168,7 @@ func (h *handler) inspect() (map[string][]lsp.Diagnostic, error) { return ret, fmt.Errorf("Failed to prepare loading: %w", err) } - configs, diags := loader.LoadConfig(".", h.config.Module) + configs, diags := loader.LoadConfig(".", h.config.CallModuleType) if diags.HasErrors() { return ret, fmt.Errorf("Failed to load configurations: %w", diags) } diff --git a/terraform/addrs/module_source.go b/terraform/addrs/module_source.go new file mode 100644 index 000000000..7b5b28317 --- /dev/null +++ b/terraform/addrs/module_source.go @@ -0,0 +1,146 @@ +package addrs + +import ( + "path" + "strings" +) + +// ModuleSource is the general type for all three of the possible module source +// address types. The concrete implementations of this are ModuleSourceLocal +// and ModuleSourceRemote. +type ModuleSource interface { + // String returns a full representation of the address, including any + // additional components that are typically implied by omission in + // user-written addresses. + // + // We typically use this longer representation in error message, in case + // the inclusion of normally-omitted components is helpful in debugging + // unexpected behavior. + String() string + + moduleSource() +} + +var _ ModuleSource = ModuleSourceLocal("") +var _ ModuleSource = ModuleSourceRemote("") + +var moduleSourceLocalPrefixes = []string{ + "./", + "../", + ".\\", + "..\\", +} + +// ParseModuleSource parses a module source address as given in the "source" +// argument inside a "module" block in the configuration. +// +// Unlike Terraform, this function only categorizes sources into "local" and "remote". +func ParseModuleSource(raw string) (ModuleSource, error) { + if isModuleSourceLocal(raw) { + localAddr, err := parseModuleSourceLocal(raw) + if err != nil { + // This is to make sure we really return a nil ModuleSource in + // this case, rather than an interface containing the zero + // value of ModuleSourceLocal. + return nil, err + } + return localAddr, nil + } + + // Return all non-local sources assuming they are remote source. + // Note that this is essentially useless for determining anything more + // than "non-local". + return ModuleSourceRemote(raw), nil +} + +// ModuleSourceLocal is a ModuleSource representing a local path reference +// from the caller's directory to the callee's directory within the same +// module package. +// +// A "module package" here means a set of modules distributed together in +// the same archive, repository, or similar. That's a significant distinction +// because we always download and cache entire module packages at once, +// and then create relative references within the same directory in order +// to ensure all modules in the package are looking at a consistent filesystem +// layout. We also assume that modules within a package are maintained together, +// which means that cross-cutting maintenence across all of them would be +// possible. +// +// The actual value of a ModuleSourceLocal is a normalized relative path using +// forward slashes, even on operating systems that have other conventions, +// because we're representing traversal within the logical filesystem +// represented by the containing package, not actually within the physical +// filesystem we unpacked the package into. We should typically not construct +// ModuleSourceLocal values directly, except in tests where we can ensure +// the value meets our assumptions. Use ParseModuleSource instead if the +// input string is not hard-coded in the program. +type ModuleSourceLocal string + +func parseModuleSourceLocal(raw string) (ModuleSourceLocal, error) { + // As long as we have a suitable prefix (detected by ParseModuleSource) + // there is no failure case for local paths: we just use the "path" + // package's cleaning logic to remove any redundant "./" and "../" + // sequences and any duplicate slashes and accept whatever that + // produces. + + // Although using backslashes (Windows-style) is non-idiomatic, we do + // allow it and just normalize it away, so the rest of Terraform will + // only see the forward-slash form. + if strings.Contains(raw, `\`) { + // Note: We use string replacement rather than filepath.ToSlash + // here because the filepath package behavior varies by current + // platform, but we want to interpret configured paths the same + // across all platforms: these are virtual paths within a module + // package, not physical filesystem paths. + raw = strings.ReplaceAll(raw, `\`, "/") + } + + // Note that we could've historically blocked using "//" in a path here + // in order to avoid confusion with the subdir syntax in remote addresses, + // but we historically just treated that as the same as a single slash + // and so we continue to do that now for compatibility. Clean strips those + // out and reduces them to just a single slash. + clean := path.Clean(raw) + + // However, we do need to keep a single "./" on the front if it isn't + // a "../" path, or else it would be ambigous with the registry address + // syntax. + if !strings.HasPrefix(clean, "../") { + clean = "./" + clean + } + + return ModuleSourceLocal(clean), nil +} + +func isModuleSourceLocal(raw string) bool { + for _, prefix := range moduleSourceLocalPrefixes { + if strings.HasPrefix(raw, prefix) { + return true + } + } + return false +} + +func (s ModuleSourceLocal) moduleSource() {} + +func (s ModuleSourceLocal) String() string { + // We assume that our underlying string was already normalized at + // construction, so we just return it verbatim. + return string(s) +} + +// ModuleSourceRemote is a ModuleSource representing a remote location from +// which we can retrieve a module package. +// +// Note that unlike Terraform, this also includes the address of the +// ModuleSourceRegistry equivalent. TFLint does not need to distinguish +// between ModuleSourceRemote and ModuleSourceRegistry, +// so they are all treated as ModuleSourceRemote. +type ModuleSourceRemote string + +func (s ModuleSourceRemote) moduleSource() {} + +func (s ModuleSourceRemote) String() string { + // The remote source is not normalized and returns the input value as-is. + return string(s) +} diff --git a/terraform/config.go b/terraform/config.go index 70e5a0bc1..0a04f7083 100644 --- a/terraform/config.go +++ b/terraform/config.go @@ -82,9 +82,10 @@ func buildChildModules(parent *Config, walker ModuleWalker) (map[string]*Config, path[len(path)-1] = call.Name req := ModuleRequest{ - Name: call.Name, - Path: path, - CallRange: call.DeclRange, + Name: call.Name, + Path: path, + SourceAddr: call.SourceAddr, + CallRange: call.DeclRange, } mod, _, modDiags := walker.LoadModule(&req) @@ -166,6 +167,10 @@ type ModuleRequest struct { // calls with the same name at different points in the tree. Path addrs.Module + // SourceAddr is the source address string provided by the user in + // configuration. + SourceAddr addrs.ModuleSource + // CallRange is the source range for the header of the "module" block // in configuration that prompted this request. This can be used as the // subject of an error diagnostic that relates to the module call itself. diff --git a/terraform/loader.go b/terraform/loader.go index b7dd85beb..3dc9aa9cf 100644 --- a/terraform/loader.go +++ b/terraform/loader.go @@ -9,6 +9,7 @@ import ( "github.com/hashicorp/go-version" "github.com/hashicorp/hcl/v2" "github.com/spf13/afero" + "github.com/terraform-linters/tflint/terraform/addrs" ) // Loader is a fork of configload.Loader. The instance is the main entry-point @@ -69,19 +70,24 @@ func NewLoader(fs afero.Afero, originalWd string) (*Loader, error) { // The second argument determines whether to load child modules. If true is given, // load installed child modules according to a manifest file. If false is given, // all child modules will not be loaded. -func (l *Loader) LoadConfig(dir string, module bool) (*Config, hcl.Diagnostics) { +func (l *Loader) LoadConfig(dir string, callModuleType CallModuleType) (*Config, hcl.Diagnostics) { mod, diags := l.parser.LoadConfigDir(l.baseDir, dir) if diags.HasErrors() { return nil, diags } var walker ModuleWalkerFunc - if module { + switch callModuleType { + case CallAllModule: log.Print("[INFO] Module inspection is enabled. Building the root module with children...") - walker = ModuleWalkerFunc(l.moduleWalkerLoad) - } else { - log.Print("[INFO] Module inspection is disabled. Building the root module without children...") - walker = ModuleWalkerFunc(l.moduleWalkerIgnore) + walker = l.moduleWalkerFunc(true, false) + case CallLocalModule: + log.Print("[INFO] Module inspection is enabled. Building the root module with children...") + walker = l.moduleWalkerFunc(true, false) + case CallNoModule: + walker = l.moduleWalkerFunc(false, false) + default: + panic(fmt.Sprintf("unexpected module call type: %d", callModuleType)) } cfg, diags := BuildConfig(mod, walker) @@ -91,35 +97,45 @@ func (l *Loader) LoadConfig(dir string, module bool) (*Config, hcl.Diagnostics) return cfg, nil } -func (l *Loader) moduleWalkerLoad(req *ModuleRequest) (*Module, *version.Version, hcl.Diagnostics) { - // Since we're just loading here, we expect that all referenced modules - // will be already installed and described in our manifest. However, we - // do verify that the manifest and the configuration are in agreement - // so that we can prompt the user to run "terraform init" if not. - - key := l.modules.manifest.moduleKey(req.Path) - record, exists := l.modules.manifest[key] - - if !exists { - log.Printf("[DEBUG] Failed to search by `%s` key.", key) - return nil, nil, hcl.Diagnostics{ - { - Severity: hcl.DiagError, - Summary: fmt.Sprintf("`%s` module is not found. Did you run `terraform init`?", req.Name), - Subject: &req.CallRange, - }, +func (l *Loader) moduleWalkerFunc(callLocal, callRemote bool) ModuleWalkerFunc { + return func(req *ModuleRequest) (*Module, *version.Version, hcl.Diagnostics) { + switch source := req.SourceAddr.(type) { + case addrs.ModuleSourceLocal: + if !callLocal { + return nil, nil, nil + } + log.Printf("[DEBUG] Trying to load the local module: name=%s dir=%s", req.Name, source.String()) + mod, diags := l.parser.LoadConfigDir(l.baseDir, source.String()) + return mod, nil, diags + + case addrs.ModuleSourceRemote: + if !callRemote { + return nil, nil, nil + } + // Since we're just loading here, we expect that all referenced modules + // will be already installed and described in our manifest. However, we + // do verify that the manifest and the configuration are in agreement + // so that we can prompt the user to run "terraform init" if not. + key := l.modules.manifest.moduleKey(req.Path) + record, exists := l.modules.manifest[key] + if !exists { + log.Printf(`[DEBUG] Failed to find "%s"`, key) + return nil, nil, hcl.Diagnostics{ + { + Severity: hcl.DiagError, + Summary: fmt.Sprintf(`"%s" module is not found. Did you run "terraform init"?`, req.Name), + Subject: &req.CallRange, + }, + } + } + log.Printf("[DEBUG] Trying to load the remote module: key=%s, version=%s, dir=%s", key, record.VersionStr, record.Dir) + mod, diags := l.parser.LoadConfigDir(l.baseDir, record.Dir) + return mod, record.Version, diags + + default: + panic(fmt.Sprintf("unexpected module source type: %T", req.SourceAddr)) } } - - log.Printf("[DEBUG] Trying to load the module: key=%s, version=%s, dir=%s", key, record.VersionStr, record.Dir) - - mod, diags := l.parser.LoadConfigDir(l.baseDir, record.Dir) - return mod, record.Version, diags -} - -func (l *Loader) moduleWalkerIgnore(req *ModuleRequest) (*Module, *version.Version, hcl.Diagnostics) { - // Prevents loading any child modules by returning nil for all module requests - return nil, nil, nil } var defaultVarsFilename = "terraform.tfvars" diff --git a/terraform/loader_test.go b/terraform/loader_test.go index f144e3ea6..8a2b46f2b 100644 --- a/terraform/loader_test.go +++ b/terraform/loader_test.go @@ -18,7 +18,7 @@ func TestLoadConfig_v0_15_0(t *testing.T) { if err != nil { t.Fatal(err) } - config, diags := loader.LoadConfig(".", true) + config, diags := loader.LoadConfig(".", CallAllModule) if diags.HasErrors() { t.Fatal(diags) } @@ -83,7 +83,7 @@ func TestLoadConfig_v0_15_0_withBaseDir(t *testing.T) { if err != nil { t.Fatal(err) } - config, diags := loader.LoadConfig(".", true) + config, diags := loader.LoadConfig(".", CallAllModule) if diags.HasErrors() { t.Fatal(diags) } @@ -147,7 +147,7 @@ func TestLoadConfig_moduleNotFound(t *testing.T) { if err != nil { t.Fatal(err) } - _, diags := loader.LoadConfig(".", true) + _, diags := loader.LoadConfig(".", CallAllModule) if !diags.HasErrors() { t.Fatal("Expected error is not occurred") } @@ -165,7 +165,7 @@ func TestLoadConfig_disableModules(t *testing.T) { if err != nil { t.Fatal(err) } - config, diags := loader.LoadConfig(".", false) + config, diags := loader.LoadConfig(".", CallNoModule) if diags.HasErrors() { t.Fatal(diags) } @@ -185,7 +185,7 @@ func TestLoadConfig_disableModules_withArgDir(t *testing.T) { if err != nil { t.Fatal(err) } - config, diags := loader.LoadConfig("before_terraform_init", false) + config, diags := loader.LoadConfig("before_terraform_init", CallNoModule) if diags.HasErrors() { t.Fatal(diags) } @@ -205,7 +205,7 @@ func TestLoadConfig_invalidConfiguration(t *testing.T) { if err != nil { t.Fatal(err) } - _, diags := loader.LoadConfig(".", false) + _, diags := loader.LoadConfig(".", CallNoModule) if !diags.HasErrors() { t.Fatal("Expected error is not occurred") } diff --git a/terraform/module_call.go b/terraform/module_call.go index 290254d5e..bfd2f174d 100644 --- a/terraform/module_call.go +++ b/terraform/module_call.go @@ -1,13 +1,17 @@ package terraform import ( + "fmt" + "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/gohcl" "github.com/terraform-linters/tflint-plugin-sdk/hclext" + "github.com/terraform-linters/tflint/terraform/addrs" ) type ModuleCall struct { Name string + SourceAddr addrs.ModuleSource SourceAddrRaw string DeclRange hcl.Range @@ -24,6 +28,19 @@ func decodeModuleBlock(block *hclext.Block) (*ModuleCall, hcl.Diagnostics) { if attr, exists := block.Body.Attributes["source"]; exists { valDiags := gohcl.DecodeExpression(attr.Expr, nil, &mc.SourceAddrRaw) diags = diags.Extend(valDiags) + + if !diags.HasErrors() { + var err error + mc.SourceAddr, err = addrs.ParseModuleSource(mc.SourceAddrRaw) + if err != nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid module source address", + Detail: fmt.Sprintf("Failed to parse module source address: %s", err), + Subject: attr.Expr.Range().Ptr(), + }) + } + } } return mc, diags @@ -36,3 +53,37 @@ var moduleBlockSchema = &hclext.BodySchema{ }, }, } + +type CallModuleType int32 + +const ( + CallAllModule CallModuleType = iota + CallLocalModule + CallNoModule +) + +func AsCallModuleType(s string) CallModuleType { + switch s { + case "all": + return CallAllModule + case "local": + return CallLocalModule + case "none": + return CallNoModule + default: + panic("never happened") + } +} + +func (c CallModuleType) String() string { + switch c { + case CallAllModule: + return "all" + case CallLocalModule: + return "local" + case CallNoModule: + return "none" + default: + panic("never happened") + } +} diff --git a/tflint/config.go b/tflint/config.go index f31c05b51..ff895b2eb 100644 --- a/tflint/config.go +++ b/tflint/config.go @@ -14,6 +14,7 @@ import ( "github.com/spf13/afero" "github.com/terraform-linters/tflint-plugin-sdk/hclext" sdk "github.com/terraform-linters/tflint-plugin-sdk/tflint" + "github.com/terraform-linters/tflint/terraform" ) var defaultConfigFile = ".tflint.hcl" @@ -59,8 +60,10 @@ var validFormats = []string{ // Config describes the behavior of TFLint type Config struct { - Module bool - ModuleSet bool + Module bool + ModuleSet bool + CallModuleType terraform.CallModuleType + CallModuleTypeSet bool Force bool ForceSet bool @@ -384,6 +387,10 @@ func (c *Config) Merge(other *Config) { c.ModuleSet = true c.Module = other.Module } + if other.CallModuleTypeSet { + c.CallModuleTypeSet = true + c.CallModuleType = other.CallModuleType + } if other.ForceSet { c.ForceSet = true c.Force = other.Force diff --git a/tflint/testing.go b/tflint/testing.go index 661df4771..a57209696 100644 --- a/tflint/testing.go +++ b/tflint/testing.go @@ -56,7 +56,7 @@ func TestRunnerWithConfig(t *testing.T, files map[string]string, config *Config) dir = dirs[0] } - configs, diags := loader.LoadConfig(dir, config.Module) + configs, diags := loader.LoadConfig(dir, config.CallModuleType) if diags.HasErrors() { t.Fatal(diags) } diff --git a/tflint/tflint_test.go b/tflint/tflint_test.go index 8c1351091..ce1b3dcb5 100644 --- a/tflint/tflint_test.go +++ b/tflint/tflint_test.go @@ -45,7 +45,7 @@ func testRunnerWithOsFs(t *testing.T, config *Config) *Runner { t.Fatal(err) } - cfg, diags := loader.LoadConfig(".", config.Module) + cfg, diags := loader.LoadConfig(".", config.CallModuleType) if diags.HasErrors() { t.Fatal(diags) } @@ -78,7 +78,7 @@ func testRunnerWithAnnotations(t *testing.T, files map[string]string, annotation t.Fatal(err) } - cfg, diags := loader.LoadConfig(".", config.Module) + cfg, diags := loader.LoadConfig(".", config.CallModuleType) if diags.HasErrors() { t.Fatal(diags) }