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.

Sign up for GitHub

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

Jump to bottom

Added terraform v14 support besides v12. #470

Merged
merged 2 commits into from
Jan 15, 2021
Merged

Added terraform v14 support besides v12. #470

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b1c5360
Added terraform v14 support
Jan 13, 2021
a359304
added unit tests for breaking changes case
Jan 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/getting-started/usage.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ Flags:
-d, --iac-dir string path to a directory containing one or more IaC files (default ".")
-f, --iac-file string path to a single IaC file
-i, --iac-type string iac type (helm, k8s, kustomize, terraform)
--iac-version string iac version (helm: v3, k8s: v1, kustomize: v3, terraform: v12)
--iac-version string iac version (helm: v3, k8s: v1, kustomize: v3, terraform: v14)
Copy link
Contributor

Choose a reason for hiding this comment

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

We are still supporting v12 as well, we should specify that in the help as well

-p, --policy-path stringArray policy path directory
-t, --policy-type strings policy type (all, aws, azure, gcp, github, k8s) (default [all])
-r, --remote-type string type of remote backend (git, s3, gcs, http)
Expand Down Expand Up @@ -240,7 +240,7 @@ $ docker run --rm --name terrascan -p 9010:9010 accurics/terrascan
Here's an example of how to send a request to the Terrascan server using curl:

``` Bash
$ curl -i -F "file=@aws_cloudfront_distribution.tf" localhost:9010/v1/terraform/v12/aws/local/file/scan
$ curl -i -F "file=@aws_cloudfront_distribution.tf" localhost:9010/v1/terraform/v14/aws/local/file/scan
HTTP/1.1 100 Continue

HTTP/1.1 200 OK
Expand Down
18 changes: 12 additions & 6 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,21 @@ module github.com/accurics/terrascan

go 1.15

replace (
k8s.io/api => k8s.io/api v0.19.0
k8s.io/apimachinery => k8s.io/apimachinery v0.19.0
k8s.io/client-go => k8s.io/client-go v0.19.0
)

require (
github.com/ghodss/yaml v1.0.0
github.com/gorilla/mux v1.8.0
github.com/hashicorp/go-cleanhttp v0.5.1
github.com/hashicorp/go-getter v1.4.2-0.20200106182914-9813cbd4eb02
github.com/hashicorp/go-getter v1.5.1
github.com/hashicorp/go-retryablehttp v0.6.6
github.com/hashicorp/go-version v1.2.0
github.com/hashicorp/hcl/v2 v2.3.0
github.com/hashicorp/terraform v0.12.28
github.com/hashicorp/hcl/v2 v2.8.2
github.com/hashicorp/terraform v0.14.4
github.com/iancoleman/strcase v0.1.3
github.com/mattn/go-isatty v0.0.8
github.com/open-policy-agent/opa v0.22.0
Expand All @@ -20,12 +26,12 @@ require (
github.com/spf13/cobra v1.0.0
github.com/zclconf/go-cty v1.7.1
go.uber.org/zap v1.16.0
golang.org/x/lint v0.0.0-20200302205851-738671d3881b // indirect
golang.org/x/mod v0.4.1 // indirect
golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f
golang.org/x/tools v0.0.0-20201201064407-fd09bd90d85c // indirect
golang.org/x/tools v0.0.0-20210114065538-d78b04bdf963 // indirect
gopkg.in/src-d/go-git.v4 v4.13.1
gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776
helm.sh/helm/v3 v3.4.0
honnef.co/go/tools v0.0.1-2020.1.6 // indirect
honnef.co/go/tools v0.1.0 // indirect
sigs.k8s.io/kustomize/api v0.7.1
)
360 changes: 223 additions & 137 deletions go.sum
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/cli/run.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ type ScanOptions struct {
// IaC type (terraform)
iacType string

// IaC version (for terraform:v12)
// IaC version (for terraform:v14)
iacVersion string

// Path to a single IaC file
Expand Down
2 changes: 1 addition & 1 deletion pkg/http-server/file-scan_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
func TestUpload(t *testing.T) {
testFilePath := "./testdata/testconfig.tf"
testIacType := "terraform"
testIacVersion := "v12"
testIacVersion := "v14"
testCloudType := "aws"
testParamName := "file"

Expand Down
2 changes: 1 addition & 1 deletion pkg/http-server/remote-repo_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func TestScanRemoteRepo(t *testing.T) {
func TestScanRemoteRepoHandler(t *testing.T) {
validRepo := "https://github.com/kanchwala-yusuf/Damn-Vulnerable-Terraform-Project.git"
testIacType := "terraform"
testIacVersion := "v12"
testIacVersion := "v14"
testCloudType := "aws"

table := []struct {
Expand Down
12 changes: 6 additions & 6 deletions pkg/http-server/testdata/testconfig.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ resource "aws_vpc" "vpc_playground" {
enable_dns_support = true
enable_dns_hostnames = true
tags = {
Environment = "${var.environment_tag}"
Environment = "var.environment_tag"
}
}

resource "aws_internet_gateway" "igw_playground" {
vpc_id = aws_vpc.vpc_playground.id
tags = {
Environment = "${var.environment_tag}"
Environment = "var.environment_tag"
}
}

Expand All @@ -24,7 +24,7 @@ resource "aws_subnet" "subnet_public_playground" {
cidr_block = var.cidr_subnet
map_public_ip_on_launch = "true"
tags = {
Environment = "${var.environment_tag}"
Environment = "var.environment_tag"
}
}

Expand All @@ -35,7 +35,7 @@ resource "aws_route_table" "rtb_public_playground" {
gateway_id = aws_internet_gateway.igw_playground.id
}
tags = {
Environment = "${var.environment_tag}"
Environment = "var.environment_tag"
}
}

Expand Down Expand Up @@ -66,7 +66,7 @@ resource "aws_security_group" "sg_playground" {
cidr_blocks = ["0.0.0.0/0"]
}
tags = {
Environment = "${var.environment_tag}"
Environment = "var.environment_tag"
}
}

Expand All @@ -82,7 +82,7 @@ resource "aws_instance" "instance_playground" {
vpc_security_group_ids = [aws_security_group.sg_playground.id]
key_name = aws_key_pair.ec2key_playground.key_name
tags = {
Environment = "${var.environment_tag}"
Environment = "var.environment_tag"
}
provisioner "remote-exec" {

Expand Down
24 changes: 22 additions & 2 deletions pkg/iac-providers/providers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"testing"

tfv12 "github.com/accurics/terrascan/pkg/iac-providers/terraform/v12"
tfv14 "github.com/accurics/terrascan/pkg/iac-providers/terraform/v14"
)

func TestNewIacProvider(t *testing.T) {
Expand All @@ -33,6 +34,13 @@ func TestNewIacProvider(t *testing.T) {
want IacProvider
wantErr error
}{
{
name: "terraform v14",
iacType: terraform,
iacVersion: terraformV14,
want: &tfv14.TfV14{},
wantErr: nil,
},
{
name: "terraform v12",
iacType: terraform,
Expand All @@ -43,7 +51,7 @@ func TestNewIacProvider(t *testing.T) {
{
name: "not supported iac type",
iacType: "not-supported",
iacVersion: terraformV12,
Copy link
Contributor

Choose a reason for hiding this comment

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

In our test code, rather than just changing all references from 12 to 14, is it possible to pass this as a parameter, then run it against multiple versions? While our code doesn't change, we just want to make sure our interaction with the library doesn't change when different versions are used either.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

iacVersion: terraformV14,
want: nil,
wantErr: errIacNotSupported,
},
Expand Down Expand Up @@ -78,7 +86,19 @@ func TestIsIacSupported(t *testing.T) {
want bool
}{
{
name: "terraform v12",
name: "terraform v14",
iacType: terraform,
iacVersion: terraformV14,
want: true,
},
{
name: "not supported iac type",
iacType: "not-supported",
iacVersion: terraformV14,
want: false,
},
{
name: "terraform v14",
iacType: terraform,
iacVersion: terraformV12,
want: true,
Expand Down
14 changes: 8 additions & 6 deletions pkg/iac-providers/register.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,15 @@ func RegisterIacProvider(iacType supportedIacType, iacVersion, defaultIacVersion
iacVersion = defaultIacVersion
}

// version support
supportedTerraformVersions := make(map[supportedIacVersion]reflect.Type)
supportedTerraformVersions[iacVersion] = iacProvider
if IacVersionMap, IacExists := supportedIacProviders[iacType]; IacExists {
IacVersionMap[iacVersion] = iacProvider
} else {
// version support
supportedIacVersions := make(map[supportedIacVersion]reflect.Type)
supportedIacVersions[iacVersion] = iacProvider
supportedIacProviders[iacType] = supportedIacVersions
}

// default version
defaultIacVersions[iacType] = defaultIacVersion

// type support
supportedIacProviders[iacType] = supportedTerraformVersions
}
11 changes: 7 additions & 4 deletions pkg/iac-providers/terraform.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,21 @@ import (
"reflect"

tfv12 "github.com/accurics/terrascan/pkg/iac-providers/terraform/v12"
tfv14 "github.com/accurics/terrascan/pkg/iac-providers/terraform/v14"
)

// terraform specific constants
const (
terraform supportedIacType = "terraform"
terraformV12 supportedIacVersion = "v12"
terraformDefaultVersion = terraformV12
terraform supportedIacType = "terraform"
terraformV14 supportedIacVersion = "v14"
terraformV12 supportedIacVersion = "v12"

terraformDefaultVersion = terraformV12
)

// register terraform as an IaC provider with terrascan
func init() {

// register iac provider
RegisterIacProvider(terraform, terraformV12, terraformDefaultVersion, reflect.TypeOf(tfv12.TfV12{}))
RegisterIacProvider(terraform, terraformV14, terraformDefaultVersion, reflect.TypeOf(tfv14.TfV14{}))
}
2 changes: 1 addition & 1 deletion pkg/iac-providers/terraform/v12/convert.go → ...ac-providers/terraform/commons/convert.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
limitations under the License.
*/

package tfv12
package commons
Copy link
Contributor

Choose a reason for hiding this comment

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

The package naming here may be misleading later on if breaking changes are introduced... These still depend on some HCL libs. If HCLv3 is added, or if we say need to support TF 0.11, I'm not sure this would fit. @kanchwala-yusuf --would it may make more sense to name this based on the HCL version supported? Does this portion of the code also depend on the tf version used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@williepaul why would be support tf 0.11 ? We'll support future versions right? And the way how golang doesn't allow to use multiple minor versions of a dependency simultaneously, I think we'll have to move with next future releases of terraform as well IMHO

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And this commons folder is inside iac-providers/terraform/. its not a general commons for all iac-providers.. So I think if we'll have to refactor things in future. It will not have impact outside of tf iac support

Copy link
Contributor

Choose a reason for hiding this comment

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

@williepaul, let's keep it simple and straight forward and not try to future proof it. I think, commons package works, and it is within terraform package itself so shouldn't be harmful

Copy link
Contributor

Choose a reason for hiding this comment

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

@dev-gaur @kanchwala-yusuf I strongly disagree with the precedent--as a scanning tool, we should aim to scan IaC of all different versions and types. Why bother having an option to specify different versions, if we don't actually support different versions? Think from a user/customer perspective, because that's most important here. If a tool I am using to scan my files all of a sudden drops support for all my 1000 v12 IaC files I already have created, what do you think I'm going to do? Upgrade all of my IaC to v14, or find a tool that works with v12? The latter would be my first choice. Enterprise users are especially concerned about how often support is dropped. Sure, you can argue they can stick with older versions of the tool, but that brings in a separate maintenance issue for us as well, because situations can arise where we need to patch older versions (such as usage of old libraries that may have been affected by a vulnerability). Additionally, updating policies becomes another issue.

When it comes to tf11, in the commercial product, we support tf11, we have some customers using tf11, so supporting tf11 may not be just an option--it may be a requirement.

As far as the naming goes, it's not that big of a deal, but my thoughts are just that when new (and incompatible) versions are released, the "commons" name will be misleading...


/*
Following code has been borrowed and modifed from:
Expand Down
2 changes: 1 addition & 1 deletion ...providers/terraform/v12/cty-converters.go → ...iders/terraform/commons/cty-converters.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
limitations under the License.
*/

package tfv12
package commons

import (
"fmt"
Expand Down
Loading