Skip to content
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

Module source pinned ref check #100

Merged
merged 9 commits into from
Apr 20, 2017
18 changes: 17 additions & 1 deletion detector/detector.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,6 @@ func IsKeyNotFound(item *ast.ObjectItem, k string) bool {
func (d *Detector) Detect() []*issue.Issue {
var issues = []*issue.Issue{}
modules := d.EvalConfig.ModuleConfig

for ruleName, creatorMethod := range detectors {
if d.Config.IgnoreRule[ruleName] {
d.Logger.Info(fmt.Sprintf("ignore rule `%s`", ruleName))
Expand All @@ -168,7 +167,24 @@ func (d *Detector) Detect() []*issue.Issue {
}
moduleDetector.Error = d.Error
moduleDetector.detect(creatorMethod, &issues)

}
}

for name, m := range modules {
if d.Config.IgnoreModule[m.Source] {
d.Logger.Info(fmt.Sprintf("ignore module `%s`", name))
continue
}
// Special case for module_pinned_source rule
modulePinnedSourceRule := "module_pinned_source"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rename to terraform_module_pinned_source :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

if d.Config.IgnoreModule[modulePinnedSourceRule] {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this d.Config.IgnoreRule ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. Changed.

d.Logger.Info(fmt.Sprintf("ignore module `%s`", modulePinnedSourceRule))
continue
}
d.Logger.Info(fmt.Sprintf("run module linter `%s`", name))
modulePinnedSourceDetector := NewModulePinnedSourceDetector(d, m.File, m.ObjectItem)
modulePinnedSourceDetector.DetectPinnedModuleSource(&issues)
}

return issues
Expand Down
94 changes: 94 additions & 0 deletions detector/module_pinned_source.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package detector

import (
"fmt"
"strings"

"github.com/hashicorp/hcl/hcl/ast"
"github.com/wata727/tflint/issue"
)

type ModulePinnedSourceDetector struct {
source string
line int
file string
}

func NewModulePinnedSourceDetector(detector *Detector, file string, item *ast.ObjectItem) *ModulePinnedSourceDetector {
sourceToken, err := hclLiteralToken(item, "source")
if err != nil {
detector.Logger.Error(err)
return nil
}
sourceText, err := detector.evalToString(sourceToken.Text)
if err != nil {
detector.Logger.Error(err)
return nil
}

return &ModulePinnedSourceDetector{
source: sourceText,
file: file,
line: sourceToken.Pos.Line,
}
}

func (d *ModulePinnedSourceDetector) DetectPinnedModuleSource(issues *[]*issue.Issue) {
lower := strings.ToLower(d.source)

if strings.Contains(lower, "git") || strings.Contains(lower, "bitbucket") {
if issue := d.detectGitSource(d.source); issue != nil {
tmp := append(*issues, issue)
*issues = tmp
}
} else if strings.HasPrefix(lower, "hg:") {
if issue := d.detectMercurialSource(d.source); issue != nil {
tmp := append(*issues, issue)
*issues = tmp
}
}
}

func (d *ModulePinnedSourceDetector) detectGitSource(source string) *issue.Issue {
if strings.Contains(source, "ref=") {
if strings.Contains(source, "ref=master") {
return &issue.Issue{
Type: issue.WARNING,
Message: fmt.Sprintf("Module source \"%s\" uses default ref \"master\"", source),
Line: d.line,
File: d.file,
}
}
} else {
return &issue.Issue{
Type: issue.WARNING,
Message: fmt.Sprintf("Module source \"%s\" is not pinned", source),
Line: d.line,
File: d.file,
}
}

return nil
}

func (d *ModulePinnedSourceDetector) detectMercurialSource(source string) *issue.Issue {
if strings.Contains(source, "rev=") {
if strings.Contains(source, "rev=default") {
return &issue.Issue{
Type: issue.WARNING,
Message: fmt.Sprintf("Module source \"%s\" uses default rev \"default\"", source),
Line: d.line,
File: d.file,
}
}
} else {
return &issue.Issue{
Type: issue.WARNING,
Message: fmt.Sprintf("Module source \"%s\" is not pinned", source),
Line: d.line,
File: d.file,
}
}

return nil
}
2 changes: 2 additions & 0 deletions docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ Issues are classified into the following three types.
- **AWS Route**
- [aws_route_not_specified_target](aws_route_not_specified_target.md)
- [aws_route_specified_multiple_targets](aws_route_specified_multiple_targets.md)
- **Terraform**
- [module_pinned_source](module_pinned_source.md)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also rename this link and title.


### Invalid Reference Issue
Report these issues if you have specified invalid resource ID, name, etc. All issues are reported as ERROR. These issues are reported when enabled deep check. In many cases, an incorrect value is specified, so please fix it.
Expand Down
35 changes: 35 additions & 0 deletions docs/module_pinned_source.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# Terraform Module Pinned Source
This issue is reported if you specify a git or mercurial repository without pinning to a non-default version.

## Example
```
module "unpinned" {
source = "git://hashicorp.com/consul.git"
}

module "default git" {
source = "git://hashicorp.com/consul.git?ref=master"
}

module "default mercurial" {
source = "hg::http://hashicorp.com/consul.hg?rev=default"
}
```

The following is the execution result of TFLint:

```
$ tflint
template.tf
WARNING:2 Module source "git://hashicorp.com/consul.git" is not pinned
WARNING:6 Module source "git://hashicorp.com/consul.git?ref=master" uses default ref "master"
WARNING:10 Module source "hg::http://hashicorp.com/consul.hg?rev=default" uses default rev "default"

Result: 3 issues (0 errors , 3 warnings , 0 notices)
```

## Why
Terraform allows you to checkout module definitions from source control. If you do not pin the version to checkout, the dependency you require may introduce major breaking changes without your awareness. To preven this, always specify an explicit version to checkout.

## How To Fix
Specify a version pin. For git repositories, it should not be "master". For Mercurial repositories, it should not be "default"
16 changes: 10 additions & 6 deletions evaluator/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@ import (
)

type hclModule struct {
Name string
Source string
Config hil.EvalConfig
Templates map[string]*hclast.File
Name string
Source string
ObjectItem *hclast.ObjectItem
File string
Config hil.EvalConfig
Templates map[string]*hclast.File
}

func detectModules(templates map[string]*hclast.File, c *config.Config) (map[string]*hclModule, error) {
Expand Down Expand Up @@ -53,8 +55,10 @@ func detectModules(templates map[string]*hclast.File, c *config.Config) (map[str
}

moduleMap[moduleKey] = &hclModule{
Name: name,
Source: moduleSource,
Name: name,
Source: moduleSource,
ObjectItem: item,
File: file,
Config: hil.EvalConfig{
GlobalScope: &hilast.BasicScope{
VarMap: varMap,
Expand Down
8 changes: 7 additions & 1 deletion evaluator/module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ module "ec2_instance" {
"960d94c2f60d34845dc3051edfad76e1": {
Name: "ec2_instance",
Source: "./tf_aws_ec2_instance",
File: "module.tf",
Config: hil.EvalConfig{
GlobalScope: &hilast.BasicScope{
VarMap: map[string]hilast.Variable{
Expand Down Expand Up @@ -73,6 +74,7 @@ module "ec2_instance" {
"960d94c2f60d34845dc3051edfad76e1": {
Name: "ec2_instance",
Source: "./tf_aws_ec2_instance",
File: "module1.tf",
Config: hil.EvalConfig{
GlobalScope: &hilast.BasicScope{
VarMap: map[string]hilast.Variable{
Expand All @@ -92,6 +94,7 @@ module "ec2_instance" {
"0cf2d4dab02de8de33c7058799b6f81e": {
Name: "ec2_instance",
Source: "github.com/wata727/example-module",
File: "module2.tf",
Config: hil.EvalConfig{
GlobalScope: &hilast.BasicScope{
VarMap: map[string]hilast.Variable{
Expand Down Expand Up @@ -141,7 +144,6 @@ module "ec2_instances" {
defer os.Chdir(prev)
testDir := dir + "/test-fixtures"
os.Chdir(testDir)

templates := make(map[string]*hclast.File)
for k, v := range tc.Input {
templates[k], _ = parser.Parse([]byte(v))
Expand All @@ -155,6 +157,10 @@ module "ec2_instances" {
t.Fatalf("\nshould not be happen error.\nError: %s\n\ntestcase: %s", err, tc.Name)
continue
}
// We don't care how the ObjectItem was created
for _, module := range result {
module.ObjectItem = nil
}

if !reflect.DeepEqual(result, tc.Result) {
t.Fatalf("\nBad: %s\nExpected: %s\n\ntestcase: %s", pp.Sprint(result), pp.Sprint(tc.Result), tc.Name)
Expand Down