diff --git a/Dockerfile b/Dockerfile index 00d09ab..c1eea3e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -26,4 +26,4 @@ COPY $APP /linter USER nobody ENTRYPOINT ["/linter"] -CMD ["lint"] \ No newline at end of file +CMD ["lint"] diff --git a/README.md b/README.md index 2056f53..80740f9 100644 --- a/README.md +++ b/README.md @@ -24,10 +24,6 @@ The workflow in '.github/workflows/generate-terraform-checksum.yml' can be used ## In a GitHub workflow ```yaml -env: - terraform_version: '1.3.2' - terraform_checksum: '${{ secrets.TERRAFORM_CHECKSUM }}' - jobs: secure-setup-terraform: runs-on: 'ubuntu-latest' @@ -38,8 +34,8 @@ jobs: name: 'secure-setup-terraform' uses: 'bradegler/secure-setup-terraform@v0.0.10' with: - terraform_version: ${{env.terraform_version}} - terraform_checksum: ${{env.terraform_checksum}} + terraform_version: '1.3.2' + terraform_checksum: '${{ secrets.TERRAFORM_CHECKSUM }}' ## Use terraform normally ``` diff --git a/cmd/lint-local-exec/linter/lint.go b/cmd/lint-local-exec/linter/lint.go index d24d896..e44a248 100644 --- a/cmd/lint-local-exec/linter/lint.go +++ b/cmd/lint-local-exec/linter/lint.go @@ -21,7 +21,7 @@ import ( "github.com/hashicorp/hcl/v2/hclsyntax" "github.com/bradegler/secure-setup-terraform/cmd/lint-local-exec/version" - "github.com/bradegler/secure-setup-terraform/lib" + "github.com/bradegler/secure-setup-terraform/pkg/lint" ) const violationType = "local-exec" @@ -32,20 +32,20 @@ type TerraformLinter struct{} // FindViolations inspects a set of bytes that represent hcl from a terraform configuration file // looking for calls to the 'local-exec' provider. -func (tfl *TerraformLinter) FindViolations(content []byte, path string) ([]lib.ViolationInstance, error) { +func (tfl *TerraformLinter) FindViolations(content []byte, path string) ([]lint.ViolationInstance, error) { tokens, diags := hclsyntax.LexConfig(content, path, hcl.Pos{Byte: 0, Line: 1, Column: 1}) if diags.HasErrors() { return nil, fmt.Errorf("error lexing hcl file contents: [%s]", diags.Error()) } - instances := []lib.ViolationInstance{} + instances := []lint.ViolationInstance{} for _, token := range tokens { if token.Type == hclsyntax.TokenQuotedLit && string(token.Bytes) == violationType { - instances = append(instances, lib.ViolationInstance{Path: path, Line: token.Range.Start.Line}) + instances = append(instances, lint.ViolationInstance{Path: path, Line: token.Range.Start.Line}) } } return instances, nil } -func (tfl *TerraformLinter) GetSelectors() []string { return selectors } -func (tfl *TerraformLinter) GetViolationType() string { return violationType } -func (tfl *TerraformLinter) GetVersion() string { return version.HumanVersion } +func (tfl *TerraformLinter) Selectors() []string { return selectors } +func (tfl *TerraformLinter) ViolationType() string { return violationType } +func (tfl *TerraformLinter) Version() string { return version.HumanVersion } diff --git a/cmd/lint-local-exec/linter/lint_test.go b/cmd/lint-local-exec/linter/lint_test.go index 6b0de39..31a52a6 100644 --- a/cmd/lint-local-exec/linter/lint_test.go +++ b/cmd/lint-local-exec/linter/lint_test.go @@ -15,31 +15,94 @@ package linter import ( - "fmt" - "reflect" "testing" - "github.com/bradegler/secure-setup-terraform/lib" + "github.com/bradegler/secure-setup-terraform/pkg/lint" + "github.com/google/go-cmp/cmp" ) func TestLint_FindViolations(t *testing.T) { t.Parallel() - withLocalExec := "resource \"google_project_service\" \"run_api\" { service = \"run.googleapis.com\" disable_on_destroy = true}resource \"google_cloud_run_service\" \"run_service\" { name = var.service_name location = var.region template { spec { containers { image = var.service_image } } } traffic { percent = 100 latest_revision = true } depends_on = [google_project_service.run_api, null_resource.echo]}resource \"null_resource\" \"echo\" { provisioner \"local-exec\" { command = \"echo this is a bad practice\" }}" - withoutLocalExec := "resource \"google_project_service\" \"run_api\" { service = \"run.googleapis.com\" disable_on_destroy = true}resource \"google_cloud_run_service\" \"run_service\" { name = var.service_name location = var.region template { spec { containers { image = var.service_image } } } traffic { percent = 100 latest_revision = true } depends_on = [google_project_service.run_api]" + withLocalExec := ` + resource "google_project_service" "run_api" { + service = "run.googleapis.com" + disable_on_destroy = true + } + resource "google_cloud_run_service" "run_service" { + name = var.service_name + location = var.region + template { + spec { + containers { + image = var.service_image + } + } + } + traffic { + percent = 100 + latest_revision = true + } + depends_on = [google_project_service.run_api, null_resource.echo] + } + resource "null_resource" "echo" { + provisioner "local-exec" { + command = "echo this is a bad practice" + } + } + ` + withoutLocalExec := ` + resource "google_project_service" "run_api" { + service = "run.googleapis.com" + disable_on_destroy = true + } + resource "google_cloud_run_service" "run_service" { + name = var.service_name + location = var.region + template { + spec { + containers { + image = var.service_image + } + } + } + traffic { + percent = 100 + latest_revision = true + } + depends_on = [google_project_service.run_api] + } + ` cases := []struct { name string filename string content string expectCount int - expect []lib.ViolationInstance + expect []lint.ViolationInstance wantError bool }{ - {name: "with local exec", filename: "/my/path/to/testfile1", content: withLocalExec, expectCount: 1, expect: []lib.ViolationInstance{ - {Path: "/my/path/to/testfile1", Line: 1}, - }, wantError: false}, - {name: "without local exec", filename: "/my/path/to/testfile2", content: withoutLocalExec, expectCount: 0, expect: nil, wantError: false}, + { + name: "with local exec", + filename: "/my/path/to/testfile1", + content: withLocalExec, + expectCount: 1, + expect: []lint.ViolationInstance{ + { + Path: "/my/path/to/testfile1", + Line: 23, + }, + }, + wantError: false, + }, + { + name: "without local exec", + filename: "/my/path/to/testfile2", + content: withoutLocalExec, + expectCount: 0, + expect: []lint.ViolationInstance{}, + wantError: false, + }, } for _, tc := range cases { @@ -47,19 +110,21 @@ func TestLint_FindViolations(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() - fmt.Printf("Test: %s\n", tc.name) l := TerraformLinter{} results, err := l.FindViolations([]byte(tc.content), tc.filename) if tc.wantError != (err != nil) { - t.Fatalf("expected error: %#v, got: %#v - %v", tc.wantError, err != nil, err) - } - if tc.expectCount != len(results) { - t.Fatalf("execpted results: %d, got: %d", tc.expectCount, len(results)) + t.Errorf("expected error: %#v, got: %#v - %v", tc.wantError, err != nil, err) } - if len(tc.expect) != 0 && !reflect.DeepEqual(results, tc.expect) { - t.Fatalf("execpted results did not match: %v, got: %v", tc.expect, results) + if diff := cmp.Diff(tc.expect, results); diff != "" { + t.Errorf("Results (-want,+got):\n%s", diff) } + // if tc.expectCount != len(results) { + // t.Fatalf("execpted results: %d, got: %d", tc.expectCount, len(results)) + // } + // if len(tc.expect) != 0 && !reflect.DeepEqual(results, tc.expect) { + // t.Fatalf("execpted results did not match: %v, got: %v", tc.expect, results) + // } }) } } diff --git a/cmd/lint-local-exec/main.go b/cmd/lint-local-exec/main.go index 55df005..4b8128f 100644 --- a/cmd/lint-local-exec/main.go +++ b/cmd/lint-local-exec/main.go @@ -22,7 +22,7 @@ import ( "syscall" "github.com/bradegler/secure-setup-terraform/cmd/lint-local-exec/linter" - "github.com/bradegler/secure-setup-terraform/lib" + "github.com/bradegler/secure-setup-terraform/pkg/lint" ) func main() { @@ -37,5 +37,5 @@ func run() error { syscall.SIGINT, syscall.SIGTERM) defer done() - return lib.RunLinter(ctx, os.Args[1:], &linter.TerraformLinter{}) + return lint.RunLinter(ctx, os.Args[1:], &linter.TerraformLinter{}) } diff --git a/cmd/lint-setup-terraform/linter/lint.go b/cmd/lint-setup-terraform/linter/lint.go index 67d49fb..939b9f4 100644 --- a/cmd/lint-setup-terraform/linter/lint.go +++ b/cmd/lint-setup-terraform/linter/lint.go @@ -21,7 +21,7 @@ import ( "strings" "github.com/bradegler/secure-setup-terraform/cmd/lint-setup-terraform/version" - "github.com/bradegler/secure-setup-terraform/lib" + "github.com/bradegler/secure-setup-terraform/pkg/lint" "gopkg.in/yaml.v3" ) @@ -34,19 +34,19 @@ type GitHubActionLinter struct{} // FindViolations inspects a set of bytes that represent a YAML document that defines // a GitHub action workflow looking for steps that use the 'hashicorp/setup-terraform' // action. -func (tfl *GitHubActionLinter) FindViolations(content []byte, path string) ([]lib.ViolationInstance, error) { +func (tfl *GitHubActionLinter) FindViolations(content []byte, path string) ([]lint.ViolationInstance, error) { reader := bytes.NewReader(content) node, err := parseYAML(reader) if err != nil { return nil, fmt.Errorf("failed to parse yaml: %w", err) } if node == nil { - return []lib.ViolationInstance{}, nil + return nil, nil } if node.Kind != yaml.DocumentNode { return nil, fmt.Errorf("expected document node, got %v", node.Kind) } - violations := []lib.ViolationInstance{} + violations := []lint.ViolationInstance{} // Top-level object map for _, docMap := range node.Content { _ = docMap @@ -85,7 +85,7 @@ func (tfl *GitHubActionLinter) FindViolations(content []byte, path string) ([]li uses := step.Content[k+1] // Looking for the specific 'hashicorp/setup-terraform' action if strings.HasPrefix(uses.Value, "hashicorp/setup-terraform") { - violations = append(violations, lib.ViolationInstance{Path: path, Line: uses.Line}) + violations = append(violations, lint.ViolationInstance{Path: path, Line: uses.Line}) } } } @@ -101,9 +101,9 @@ func (tfl *GitHubActionLinter) FindViolations(content []byte, path string) ([]li return violations, nil } -func (tfl *GitHubActionLinter) GetSelectors() []string { return selectors } -func (tfl *GitHubActionLinter) GetViolationType() string { return violationType } -func (tfl *GitHubActionLinter) GetVersion() string { return version.HumanVersion } +func (tfl *GitHubActionLinter) Selectors() []string { return selectors } +func (tfl *GitHubActionLinter) ViolationType() string { return violationType } +func (tfl *GitHubActionLinter) Version() string { return version.HumanVersion } // parseYAML parses the given reader as a yaml node. func parseYAML(r io.Reader) (*yaml.Node, error) { diff --git a/cmd/lint-setup-terraform/linter/lint_test.go b/cmd/lint-setup-terraform/linter/lint_test.go index 667ff87..e056745 100644 --- a/cmd/lint-setup-terraform/linter/lint_test.go +++ b/cmd/lint-setup-terraform/linter/lint_test.go @@ -16,10 +16,10 @@ package linter import ( "fmt" - "reflect" "testing" - "github.com/bradegler/secure-setup-terraform/lib" + "github.com/bradegler/secure-setup-terraform/pkg/lint" + "github.com/google/go-cmp/cmp" ) func TestLint_FindViolations(t *testing.T) { @@ -93,11 +93,30 @@ jobs: filename string content string expectCount int - expect []lib.ViolationInstance + expect []lint.ViolationInstance wantError bool }{ - {name: "yaml without setup-terraform action", filename: "/test/myfile1", content: withoutSetupTerraform, expectCount: 0, expect: nil, wantError: false}, - {name: "yaml with setup-terraform action", filename: "/test/myfile2", content: withSetupTerraform, expectCount: 1, expect: []lib.ViolationInstance{{Path: "/test/myfile2", Line: 31}}, wantError: false}, + { + name: "yaml without setup-terraform action", + filename: "/test/myfile1", + content: withoutSetupTerraform, + expectCount: 0, + expect: []lint.ViolationInstance{}, + wantError: false, + }, + { + name: "yaml with setup-terraform action", + filename: "/test/myfile2", + content: withSetupTerraform, + expectCount: 1, + expect: []lint.ViolationInstance{ + { + Path: "/test/myfile2", + Line: 31, + }, + }, + wantError: false, + }, } for _, tc := range cases { @@ -110,13 +129,10 @@ jobs: l := GitHubActionLinter{} results, err := l.FindViolations([]byte(tc.content), tc.filename) if tc.wantError != (err != nil) { - t.Fatalf("expected error: %#v, got: %#v - %v", tc.wantError, err != nil, err) + t.Errorf("expected error: %#v, got: %#v - %v", tc.wantError, err != nil, err) } - if tc.expectCount != len(results) { - t.Fatalf("execpted results: %d, got: %d", tc.expectCount, len(results)) - } - if len(tc.expect) != 0 && !reflect.DeepEqual(results, tc.expect) { - t.Fatalf("execpted results did not match: %v, got: %v", tc.expect, results) + if diff := cmp.Diff(tc.expect, results); diff != "" { + t.Errorf("Results (-want,+got):\n%s", diff) } }) } diff --git a/cmd/lint-setup-terraform/main.go b/cmd/lint-setup-terraform/main.go index 0f4a775..6af9494 100644 --- a/cmd/lint-setup-terraform/main.go +++ b/cmd/lint-setup-terraform/main.go @@ -22,7 +22,7 @@ import ( "syscall" "github.com/bradegler/secure-setup-terraform/cmd/lint-setup-terraform/linter" - "github.com/bradegler/secure-setup-terraform/lib" + "github.com/bradegler/secure-setup-terraform/pkg/lint" ) func main() { @@ -37,5 +37,5 @@ func run() error { syscall.SIGINT, syscall.SIGTERM) defer done() - return lib.RunLinter(ctx, os.Args[1:], &linter.GitHubActionLinter{}) + return lint.RunLinter(ctx, os.Args[1:], &linter.GitHubActionLinter{}) } diff --git a/go.mod b/go.mod index ac1ba72..227fdeb 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,10 @@ module github.com/bradegler/secure-setup-terraform go 1.18 -require github.com/hashicorp/hcl/v2 v2.14.1 +require ( + github.com/google/go-cmp v0.3.1 + github.com/hashicorp/hcl/v2 v2.14.1 +) require ( github.com/agext/levenshtein v1.2.1 // indirect diff --git a/package-lock.json b/package-lock.json deleted file mode 100644 index 7b9ddda..0000000 --- a/package-lock.json +++ /dev/null @@ -1,13 +0,0 @@ -{ - "name": "run-vertexai-notebook", - "version": "0.0.2", - "lockfileVersion": 2, - "requires": true, - "packages": { - "": { - "name": "run-vertexai-notebook", - "version": "0.0.2", - "license": "Apache-2.0" - } - } -} diff --git a/package.json b/package.json deleted file mode 100644 index b71bc89..0000000 --- a/package.json +++ /dev/null @@ -1,20 +0,0 @@ -{ - "name": "secure-setup-terraform", - "version": "0.0.1", - "description": "This works with our versioning tools, this is NOT an NPM repo", - "scripts": { - "build": "echo \"No build required for composite action\"", - "test": "echo \"Error: no test specified\" && exit 1" - }, - "repository": { - "type": "git", - "url": "git+https://github.com/bradegler/secure-setup-terraform.git" - }, - "keywords": [], - "author": "", - "license": "Apache-2.0", - "bugs": { - "url": "https://github.com/bradegler/secure-setup-terraform/issues" - }, - "homepage": "https://github.com/bradegler/secure-setup-terraform#readme" -} diff --git a/lib/lint_command.go b/pkg/lint/lint_command.go similarity index 74% rename from lib/lint_command.go rename to pkg/lint/lint_command.go index 00d1ac1..c7fced6 100644 --- a/lib/lint_command.go +++ b/pkg/lint/lint_command.go @@ -1,4 +1,4 @@ -package lib +package lint import ( "context" @@ -15,11 +15,11 @@ EXAMPLES FLAGS ` -// Run executes the linter for a set of files +// RunLinter run executes the linter for a set of files func RunLinter(ctx context.Context, originalArgs []string, linter Linter) error { f := flag.NewFlagSet("", flag.ExitOnError) f.Usage = func() { - fmt.Fprintf(os.Stderr, "%s\n\n", strings.TrimSpace(fmt.Sprintf(lintCommandHelp, linter.GetViolationType()))) + fmt.Fprintf(os.Stderr, "%s\n\n", strings.TrimSpace(fmt.Sprintf(lintCommandHelp, linter.ViolationType()))) f.PrintDefaults() } showVersion := f.Bool("version", false, "display version information") @@ -28,7 +28,7 @@ func RunLinter(ctx context.Context, originalArgs []string, linter Linter) error return fmt.Errorf("failed to parse flags: %w", err) } if *showVersion { - fmt.Fprintln(os.Stderr, linter.GetVersion()) + fmt.Fprintln(os.Stderr, linter.Version()) return nil } @@ -38,19 +38,17 @@ func RunLinter(ctx context.Context, originalArgs []string, linter Linter) error return fmt.Errorf("expected atleast one argument, got %d", got) } - // Wrap the linter with a controller that can manage files / directories - fileLinter := NewFileLinter(linter) violations := []ViolationInstance{} // Process each provided path looking for violations for _, path := range args { - instances, err := fileLinter.Lint(path) + instances, err := lint(path, linter) if err != nil { return fmt.Errorf("error linting files: %w", err) } violations = append(violations, instances...) } for _, instance := range violations { - fmt.Printf("'%s' detected at [%s:%d]\n", linter.GetViolationType(), instance.Path, instance.Line) + fmt.Printf("'%s' detected at [%s:%d]\n", linter.ViolationType(), instance.Path, instance.Line) } if len(violations) != 0 { return fmt.Errorf("found %d violation(s)", len(violations)) diff --git a/lib/file_linter.go b/pkg/lint/linter.go similarity index 56% rename from lib/file_linter.go rename to pkg/lint/linter.go index 4b80d1c..748871c 100644 --- a/lib/file_linter.go +++ b/pkg/lint/linter.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package lib +package lint import ( "fmt" @@ -22,44 +22,41 @@ import ( "strings" ) +// ViolationInstance is an object that contains a reference to a location +// in a file where a lint violation was detected. type ViolationInstance struct { Path string Line int } +// Linter defines an interface selecting a set of files to apply lint rules +// against. type Linter interface { - GetViolationType() string - GetVersion() string + // ViolationType declears the type of violation the Linter identifies + ViolationType() string + // Version retrieves the human readable version string for the linter + Version() string + // Selectors provides a set of file suffixes to search for. '.tf', '.yml', etc. + Selectors() []string + // FindViolations is the specific linter implementation that is applied to each + // file to find any lint violations. FindViolations(content []byte, path string) ([]ViolationInstance, error) - GetSelectors() []string } -type FileLinter interface { - Lint(filePath string) ([]ViolationInstance, error) -} - -type fileLinter struct { - Linter Linter -} - -func NewFileLinter(linter Linter) FileLinter { - return &fileLinter{Linter: linter} -} - -func (fl *fileLinter) Lint(path string) ([]ViolationInstance, error) { +func lint(path string, linter Linter) ([]ViolationInstance, error) { isDir, err := isDirectory(path) if err != nil { - return nil, fmt.Errorf("error reading file at path [%s]: %w", path, err) + return nil, fmt.Errorf("error reading file at path %q: %w", path, err) } + instances := []ViolationInstance{} if isDir { files, err := ioutil.ReadDir(path) if err != nil { - return nil, fmt.Errorf("error reading directory at path [%s]: %w", path, err) + return nil, fmt.Errorf("error reading directory at path %q: %w", path, err) } - instances := []ViolationInstance{} for _, file := range files { next := filepath.Join(path, file.Name()) - results, err := fl.Lint(next) + results, err := lint(next, linter) if err != nil { return nil, err } @@ -69,17 +66,21 @@ func (fl *fileLinter) Lint(path string) ([]ViolationInstance, error) { } return instances, err } else { - for _, sel := range fl.Linter.GetSelectors() { + for _, sel := range linter.Selectors() { if strings.HasSuffix(path, sel) { content, err := ioutil.ReadFile(path) if err != nil { return nil, fmt.Errorf("error reading file: [%w]", err) } - return fl.Linter.FindViolations(content, path) + results, err := linter.FindViolations(content, path) + if err != nil { + return nil, err + } + instances = append(instances, results...) } } } - return nil, nil + return instances, nil } func isDirectory(path string) (bool, error) {