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

Deep modules location mis-proccessed. #365

Closed
salexpdx opened this issue Oct 29, 2020 · 3 comments · Fixed by #372 or #373
Closed

Deep modules location mis-proccessed. #365

salexpdx opened this issue Oct 29, 2020 · 3 comments · Fixed by #372 or #373

Comments

@salexpdx
Copy link

  • terrascan version: Latest (2c964d5)
  • Operating System: OS X (Darwin MacBook-Pro.local 18.7.0 Darwin Kernel Version 18.7.0: Mon Aug 31 20:53:32 PDT 2020; root:xnu-4903.278.44~1/RELEASE_X86_64 x86_64)

Description

I have a situation where I am using several modules together. For simplicity, we will just call them m1, m2, and m3. m1 is consumed from my main terraform template and it consumes m2 and m3. When I try and run terrascan, it incorrectly parsers the directory in the module as relative to absRootDir instead of relative to the location of m1.

The file system would look like this
/tf/template.tf
/tf/modules/m1/main.tf
/tf/modules/m2/main.tf

/tf/template.tf contains a call like:

module "m1" {
    source "./modules/m1/"
}

/tf/modules/m1/main.tf contains a call like

module "m2" {
    source "../m2/"
}

That works just fine in terraform and it parses them correctly. 

What I Did

When I run terrascan on the directory though, I get the following error

$ terrascan scan -t aws 
2020-10-29T10:08:46.046-0700	error	v12/load-dir.go:110	failed to build unified config. errors:
<nil>: Failed to read module directory; Module directory /tf/m2 does not exist or cannot be read.

I tossed some debugging in to see how it is parsing it internally into the module walker function:

2020/10/29 10:16:11 req.Path.String(), is m1
2020/10/29 10:16:11 req.SourceAddr, is ./modules/m1
2020/10/29 10:16:11 Created path to module as  /tf/modules/m1
2020/10/29 10:16:11 processing local module "./modules/m1"
2020/10/29 10:16:11 req.Path.String(), is m1.m2
2020/10/29 10:16:11 req.SourceAddr, is ../m2
2020/10/29 10:16:11 Created path to module as  /tf/m3
2020/10/29 10:16:11 processing local module "../m2"

The code added at the end of the if statement at line 84 in pkg/iac-providers/terraform/v12/load-dir.go to generate the above is:

log.Println("req.Path.String(), is", req.Path.String())
log.Println("req.SourceAddr, is", req.SourceAddr)
log.Println("Created path to module as ", pathToModule)
log.Printf("processing local module %q", req.SourceAddr)

I think the mistake is this code here (still in the if statement that starts at 84):

pathArr := strings.Split(req.Path.String(), ".")
pathArr = pathArr[:len(pathArr)-1]
pathToModule = filepath.Join(absRootDir, filepath.Join(pathArr...), req.SourceAddr)

Instead of looking at the req.Path.String, it should be looking at req.Parent.SourceAddr. If that were used instead, it would result in pathToModules containing the following values:

absRootDir = "/tf/"
req.Parrent.SourceAddr = "./modules/m1"
req.SourceAddr = "../m2"

which would get combined with the filepath.Join to result in

pathToModule = "/tf/modules/m2"
salexpdx added a commit to salexpdx/terrascan that referenced this issue Oct 30, 2020
@salexpdx
Copy link
Author

The commit above is a working fix, I haven't had a chance to look at how you are handling testing in here so I haven't created a PR.

@kanchwala-yusuf
Copy link
Contributor

Hi @salexpdx ,

We had an exact same issue in the past and the PR raised by @guilhem for the same.

Moreover, I tried the same scenario again on my local setup with the latest master. I am not able reproduce the same.

Here's how my directory structure looks lik:

~ tree tf-example
tf-example
├── cloudfront
│   └── main.tf
└── root
    └── main.tf

root/main.tf:

~ cat tf-example/root/main.tf
provider "aws" {
  region = "us-east-1"
}

module "cloudfront" {
  source = "../cloudfront"
}

cloudfront/main.tf:

~ cat tf-example/cloudfront/main.tf
resource "aws_cloudfront_distribution" "s3-distribution-TLS-v1" {
  origin {
    domain_name = "aws_s3_bucket.b.bucket_regional_domain_name"
    origin_id   = "local.s3_origin_id"

    s3_origin_config {
      origin_access_identity = "origin-access-identity/cloudfront/ABCDEFG1234567"
    }
  }

  enabled = true

  default_cache_behavior {
    allowed_methods  = ["DELETE", "GET", "HEAD", "OPTIONS", "PATCH", "POST", "PUT"]
    cached_methods   = ["GET", "HEAD"]
    target_origin_id = "local.s3_origin_id"

    forwarded_values {
      query_string = false

      cookies {
        forward = "none"
      }
    }
    viewer_protocol_policy = "https-only"
  }

  ordered_cache_behavior {
    path_pattern     = "/content/immutable/*"
    allowed_methods  = ["GET", "HEAD", "OPTIONS"]
    cached_methods   = ["GET", "HEAD", "OPTIONS"]
    target_origin_id = "local.s3_origin_id"

    forwarded_values {
      query_string = false
      headers      = ["Origin"]

      cookies {
        forward = "none"
      }
    }

    compress               = true
    viewer_protocol_policy = "allow-all"
  }

  ordered_cache_behavior {
    path_pattern     = "/content/*"
    allowed_methods  = ["GET", "HEAD", "OPTIONS"]
    cached_methods   = ["GET", "HEAD"]
    target_origin_id = "local.s3_origin_id"

    forwarded_values {
      query_string = false

      cookies {
        forward = "none"
      }
    }

    viewer_protocol_policy = "allow-all"
  }

  restrictions {
    geo_restriction {
      restriction_type = "whitelist"
      locations        = ["US", "CA", "GB", "DE"]
    }
  }

  viewer_certificate {
    cloudfront_default_certificate = true
    minimum_protocol_version       = "TLSv1" #expected version is TLSv1.1 or TLSv1.2
  }
}

locals {
  s3_origin_id = "myS3Origin"
}

Running terrascan on this directory structure:

~ terrascan scan -t aws -d ~/tf-example/root
results:
  violations:
  - rule_name: cloudfrontNoHTTPSTraffic
    description: Use encrypted connection between CloudFront and origin server
    rule_id: AWS.CloudFront.EncryptionandKeyManagement.High.0407
    severity: HIGH
    category: Encryption and Key Management
    resource_name: s3-distribution-TLS-v1
    resource_type: aws_cloudfront_distribution
    file: ../cloudfront/main.tf
    line: 1
  - rule_name: cloudfrontNoHTTPSTraffic
    description: Use encrypted connection between CloudFront and origin server
    rule_id: AWS.CloudFront.EncryptionandKeyManagement.High.0407
    severity: HIGH
    category: Encryption and Key Management
    resource_name: s3-distribution-TLS-v1
    resource_type: aws_cloudfront_distribution
    file: ../cloudfront/main.tf
    line: 1
  - rule_name: cloudfrontNoLogging
    description: Ensure that your AWS Cloudfront distributions have the Logging feature enabled in order to track all viewer requests for the content delivered through the Content Delivery Network (CDN).
    rule_id: AWS.CloudFront.Logging.Medium.0567
    severity: MEDIUM
    category: Logging
    resource_name: s3-distribution-TLS-v1
    resource_type: aws_cloudfront_distribution
    file: ../cloudfront/main.tf
    line: 1
  count:
    low: 0
    medium: 1
    high: 2
    total: 3

@salexpdx
Copy link
Author

That example only shows a single module depth. I created a quick test case that shows the issue when you embed a module within a module. salexpdx@68c7601

terraform init

Initializing modules...
- m1.m2 in modules/m2
- m1.m3 in modules/m3

Initializing the backend...

Initializing provider plugins...
- Checking for available provider plugins...
- Downloading plugin for provider "aws" (hashicorp/aws) 2.58.0...

Terraform has been successfully initialized!

You may now begin working with Terraform. Try running "terraform plan" to see
any changes that are required for your infrastructure. All Terraform commands
should now work.

If you ever set or change modules or backend configuration for Terraform,
rerun this command to reinitialize your working directory. If you forget, other
commands will detect it and remind you to do so if necessary.

terraform plan

Refreshing Terraform state in-memory prior to plan...
The refreshed state will be used to calculate this plan, but will not be
persisted to local or remote state storage.

module.m1.module.m2.data.aws_iam_policy_document.readbuckets: Refreshing state...

------------------------------------------------------------------------

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # module.m1.aws_s3_bucket.bucket will be created
  + resource "aws_s3_bucket" "bucket" {
      + acceleration_status         = (known after apply)
      + acl                         = "private"
      + arn                         = (known after apply)
      + bucket                      = "tf-test-project-dev"
      + bucket_domain_name          = (known after apply)
      + bucket_regional_domain_name = (known after apply)
      + force_destroy               = false
      + hosted_zone_id              = (known after apply)
      + id                          = (known after apply)
      + policy                      = jsonencode(
            {
              + Statement = [
                  + {
                      + Action    = [
                          + "s3:GetObject",
                        ]
                      + Effect    = "Allow"
                      + Principal = "*"
                      + Resource  = [
                          + "arn:aws:s3:::tf-test-project-dev/*",
                        ]
                      + Sid       = "PublicRead"
                    },
                ]
              + Version   = "2012-10-17"
            }
        )
      + region                      = (known after apply)
      + request_payer               = (known after apply)
      + website_domain              = (known after apply)
      + website_endpoint            = (known after apply)

      + versioning {
          + enabled    = (known after apply)
          + mfa_delete = (known after apply)
        }
    }

Plan: 1 to add, 0 to change, 0 to destroy.

------------------------------------------------------------------------

Note: You didn't specify an "-out" parameter to save this plan, so Terraform
can't guarantee that exactly these actions will be performed if
"terraform apply" is subsequently run.

terrascan output

../../../../../../bin/terrascan scan -t aws
2020-10-30T09:33:46.429-0700	error	v12/load-dir.go:110	failed to build unified config. errors:
<nil>: Failed to read module directory; Module directory /.../terrascan/pkg/iac-providers/terraform/v12/testdata/deep-modules/m2 does not exist or cannot be read., and 1 other diagnostic(s)

acc-jon added a commit to acc-jon/terrascan that referenced this issue Nov 6, 2020
@acc-jon acc-jon linked a pull request Nov 6, 2020 that will close this issue
kanchwala-yusuf added a commit that referenced this issue Nov 6, 2020
Address #365 by properly handling submodule path
acc-jon pushed a commit to acc-jon/terrascan that referenced this issue Nov 6, 2020
@acc-jon acc-jon linked a pull request Nov 6, 2020 that will close this issue
kanchwala-yusuf added a commit that referenced this issue Nov 10, 2020
properly handle nested submodules (#365)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants