hashicorp / terraform

Terraform enables you to safely and predictably create, change, and improve infrastructure. It is a source-available tool that codifies APIs into declarative configuration files that can be shared amongst team members, treated as code, edited, reviewed, and versioned.
https://www.terraform.io/
Other
42.31k stars 9.49k forks source link

Problem with dependant module resolution if the path is relative #23333

Closed xocasdashdash closed 1 year ago

xocasdashdash commented 4 years ago

Terraform Version

0.12.13

Terraform Configuration Files

Here you can see two examples: https://github.com/xocasdashdash/terraform-test-case

One works perfectly with 0.11, same one fails on 0.12.13 (and on dev too).

Debug Output

2019/11/09 10:56:07 [INFO] Terraform version: 0.12.13
2019/11/09 10:56:07 [INFO] Go runtime version: go1.12.9
2019/11/09 10:56:07 [INFO] CLI args: []string{"/usr/local/Cellar/tfenv/0.6.0/versions/0.12.13/terraform", "init"}
2019/11/09 10:56:07 [DEBUG] Attempting to open CLI config file: /Users/joaquin.fernandez/.terraformrc
2019/11/09 10:56:07 [DEBUG] File doesn't exist, but doesn't need to. Ignoring.
2019/11/09 10:56:07 [DEBUG] checking for credentials in "/Users/joaquin.fernandez/.terraform.d/plugins"
2019/11/09 10:56:07 [DEBUG] checking for credentials in "/Users/joaquin.fernandez/.terraform.d/plugins/darwin_amd64"
2019/11/09 10:56:07 [INFO] CLI command args: []string{"init"}
2019/11/09 10:56:07 [TRACE] ModuleInstaller: installing child modules for . into .terraform/modules
Initializing modules...
2019/11/09 10:56:07 [DEBUG] Module installer: begin a-module
2019/11/09 10:56:07 [TRACE] ModuleInstaller: Module installer: a-module <nil> already installed in .terraform/modules/a-module
2019/11/09 10:56:07 [DEBUG] Module installer: begin a-module.b_module
2019/11/09 10:56:07 [TRACE] ModuleInstaller: Module installer: a-module.b_module <nil> already installed in /Users/joaquin.fernandez/projects/personal/terraform-test/not-works-on-tf-0.12.13/modules/a-module/b-module
2019/11/09 10:56:07 [DEBUG] Module installer: begin a-module.b_module.c_module
2019/11/09 10:56:07 [TRACE] ModuleInstaller: Module installer: a-module.b_module.c_module <nil> already installed in /Users/joaquin.fernandez/projects/personal/terraform-test/not-works-on-tf-0.12.13/modules/a-module/c-module
2019/11/09 10:56:07 [DEBUG] Module installer: begin a-module.d_module
2019/11/09 10:56:07 [TRACE] ModuleInstaller: a-module.d_module has local path "../d-module/"
2019/11/09 10:56:07 [TRACE] ModuleInstaller: a-module.d_module uses directory from parent: .terraform/modules/d-module
2019/11/09 10:56:07 [DEBUG] Module installer: a-module.d_module installed at
2019/11/09 10:56:07 [TRACE] modsdir: writing modules manifest to .terraform/modules/modules.json
- a-module.d_module in

Error: Unreadable module directory

Unable to evaluate directory symlink: lstat .terraform/modules/d-module: no
such file or directory

Error: Failed to read module directory

Module directory  does not exist or cannot be read.

Error: Unreadable module directory

Unable to evaluate directory symlink: lstat .terraform/modules/d-module: no
such file or directory

Error: Failed to read module directory

Expected Behavior

It should resolve to the correct module path for the "d-module".

Actual Behavior

It does not. But if I change the route to use a local symlink and add a double "//" on the last folder before the module folder "//a-module" and set up a symlink from the module to the parent folder it does work correctly.

Steps to Reproduce

Run terraform init in each of the three folders with the last working version (0.11.14 and 0.12.13).

Additional Context

I've tried to fix it myself and I think the fix should go to this function: https://github.com/hashicorp/terraform/blob/6f66aad03262441521829ca3a678da2bb6bf51d9/internal/initwd/module_install.go#L226

I'm gonna try some more to make it work but I believe a bigger change will be needed to get this to work in all cases

danieldreier commented 4 years ago

I have confirmed that the github reproduction cases are valid with 0.12.17. It looks to me like this only happens when the relative path is traversed upward (..). The defect appears to be that upward traversal of relative paths in dependent module resolution is broken. Relative paths (eg source = "./b-module/") work as expected.

@xocasdashdash how much impact does this have on your ability to use terraform? I'm interested in other people's input as well in order to prioritize this. @xocasdashdash are you still interested in trying to fix this yourself?

xocasdashdash commented 4 years ago

So i managed to work around the issue by changing to using a symlink and that works fine. I honestly think this would be really hard to "fix". It might be better to just document it and maybe improve the error handling. In my experience most of the time you can change an upwards traversal (../) with a symlink and that works fine.

After looking at the code that does module installation, I think it could do with some cleanup (Probably the tests the most). I'm afraid about doing any change, as it might "break" module resolution for some corner case.

So to sum up I think this needs:

  1. Documentating this behaviour
  2. Improving the error message maybe suggesting the change to a symlink.
NiyiOdumosu commented 4 years ago

Can you please outline the steps you did to create a symlink for this? I ran into the exact same issue with this version of terraform and it has been driving me insane @xocasdashdash

xocasdashdash commented 4 years ago

Yeah sure. Basically I have a module called "tf-modules/stacks/elastic_search", that module used to depend on a module on a path that was "../../instance". Basically I created a symlink like this

ln -s ../../instance instance

And in the terraform code i have this:

module "elastic_search_master" {
  source              = "./instance"

Not much more.

And yes, it can drive you very very insane very quick

NiyiOdumosu commented 4 years ago

thanks man! appreciate it

brianabston001 commented 4 years ago

I am also upgrading from 11.14 to 12.18 and I am having issues with upwards traversal (../) in module sources. I have a lot of modules and making symlinks is not an option for us. Is this something that is not supported anymore?

brianabston001 commented 4 years ago

sounds like the way we are running this has changed. It has to be run from the root and init on the workspace dir.

terraform init environment/stage

before i was able to run from the base dir environment/stage terraform init

Sounds like this is different since we are running local with a remote backend.

Sushruth-Godugunuari commented 4 years ago

@danieldreier - +1 on the impacted list, we have quite a few modules and setting up symlinks is not practical

augustorosa commented 4 years ago

+1 for me. Too many modules for symlinks.

a30001539 commented 4 years ago

+1 on the impacted list

we have a number of terraform projects, with local modules arching over them all. Every project makes an "ascending path call" typically of the form

module "globals" {
  # output-only module
  source = "../../modules/globals"
}

module "environment" {
  # some interpolation done here
  source = "../../modules/environment"
  environment = local.environment

module/globals has one file in it module/globals/output.tf that is of the form

output "tags" {
    value = {
        BusinessOwner  = "AGL"
        TechnicalOwner = "DevOps"
        CostCentre     = "CCC001"
        CreatedBy      = "DevOps Angel"
        Project        = "OneCodeBase"
    }
}

output tenant_id {
    value = "74f9ac2f-c1d2-412f-8435-6e60efdad5e1"
}

...

const/environment is more usual, with main.tf, output.tf and a variables.tf const/environment/main.tf is of the form

locals {
    subscriptions = {
        default = ""
        dev = "XXXXXXXX-XXX-XXXX-XXXX-XXXXXXXXXXXX"
        prd = "YYYYYYYY-YYY-YYYY-YYYY-YYYYYYYYYYYY"
    }

    locations = {
        default = ""
        dev = "australiasoutheast"
        prd = "australiaeast"
    }
}

and the modules/environment/output.tf is of the form

output "subscription" {
    value = "${local.subscriptions[var.environment]}"
}

output "location" {
    value = "${local.locations[var.environment]}"
}

Then each project can just abstract away the specifics, and changes can be centralised

provider "azurerm" {
  tenant_id       = module.global.tenant_id
  subscription_id = module.environment.subscription
  version         = "~> 1.0"
}

This helps enormously if you use workspace prefixes

terraform {
  backend "remote" {
    hostname     = "app.terraform.io"
    organization = "XXXXXX"

    workspaces {
      prefix = "enterprise-data-"
    }
  }
}
sean-abbott commented 4 years ago

Can confirm in 12.20, and this is a big impact for us.

shukla2009 commented 4 years ago

can confirm with terraform 12.19 and its pain

usernolan commented 4 years ago

Also running into this in 0.12.19 and 0.12.21. It doesn't appear that any upward traversal is happening (at least explicitly). We don't have any ".." in our module paths.

sean-abbott commented 4 years ago

Still happening in 12.23, and it is almost random. I have 3 modules I'm referencing from the same root module, all useing the workaround pattern of softlinking, and ONE of them is failing with "cannot lstat", but the other 2 are fine.

Manually linking the local version of the submodule works, and FML.

chipzzz commented 4 years ago

+1 12.24 still there

dgreenfield0 commented 4 years ago

+1 - Terraform v0.12.24

danny-zorroa commented 4 years ago

+1 having a big impact on our workflow.

cappetta commented 4 years ago

+1 for impact, just caught this in my cicd pipeline.

cappetta commented 4 years ago

Thinking outloud - I don't understand the linking suggestion. I'm wondering if we need to flatten out the folder structure. I'm not sure how to evolve around the error. My pipeline has terraform being checked/executed with each pushed set of changes.

xocasdashdash commented 4 years ago

@cappetta Have you checked out the test case i posted? Here you can see how the symlink is used to work around the issue

cappetta commented 4 years ago

@xocasdashdash - Yes, I believe so. This is referring to the https://github.com/hashicorp/terraform/issues/23333#issuecomment-564662878 comment? If yes, I was working on this last night and trying to setup / fix the symlink issue yet it was not working for me.

Keep me honest, in the folder of the manifest which declares/calls the module '../../xx-module' you are providing a soft-link to the folder so ./xx-module-link -> ../../xx-module then updating the manifest to perform a module lookup in the current directory './xx-module-link'

if I misunderstood or confused you please recap how you worked around this. many thanks in advance

xocasdashdash commented 4 years ago

@cappetta Sorry, I forgot to copy the link. If you go to this test case https://github.com/xocasdashdash/terraform-test-case/blob/master/works-on-tf-0.12.13/modules/a-module/main.tf you can see how I managed to work around the issue

cappetta commented 4 years ago

@xocasdashdash - Excellent job, thanks for sharing!

a30001539 commented 4 years ago

I also work around this by symlinking, something I didnt have to do in tf 0.11

rawrgulmuffins commented 4 years ago

I hit this today on 0.12.26. It's moderately impacting for us. It means that for local testing we have to symlink all of our shared dependency modules (common-utils, compositional libraries, common alarms, etc.) into all of our modules. More annoying than breaking once you know what's going on but you have to find this ticket first to understand why you're breaking.

abhijeetka commented 4 years ago

same issue while I am working on Terrafrom 0.12.24 version.

kbroughton commented 4 years ago

still true... Terraform 0.12.29.

It appears to be an OSX issue.

I'm working with terraform-google-modules which relies heavily on "../../modules", and I'm getting a wide variety of errors. This one comes up often. If I switch to a container and add a bunch to the default hashicorp/terraform (bash, python3, curl) then I can run the "test/setup" folder. The symlink workaround would be a large refactor cause nearly every file has "../../modules".

It seems like the fix would be to eliminate the dependency on lstat (at least for osx) and fine another means to test for files. stat is present on osx, but not lstat.

Valen-Sentrosi commented 4 years ago

+1 still an issue on 0.12.29 and 0.13.0. Not an option for me to workaround with symlinking.

woz5999 commented 4 years ago

+1 getting this on Terraform v0.13.1

Edit: some case-specific behaviors from my experience:

If I use an absolute path from my project dir to the module dir, I get this error. If I use a relative path to the module dir, everything works fine. Obviously switching to use relative paths from projects to module sources isn't a viable workaround for everyone, but it's another potential option for those who can swing it.

andywatts commented 4 years ago

double slash "//" after repo name fixed mine. source = "../../../../../../../common-terraform//modules/data/aurora-postgres

schollii commented 3 years ago

I ran into this with 0.13.2 and can confirm as @woz5999 said, but there are 2 workarounds. Eg given the following

/root-tf
    main.tf
       module "b" { 
          source = /tf-modules/B
          ...
       }
/tf-modules
    C
        maint.tf
    B
        main.tf
            module "c" { 
                source = ../C
                ...
            }

terraform apply will cause the error to occur. Now either one of the following will NOT cause the error:

  1. in /root-tf/main.tf, change source to ../tf-modules/B
  2. in /tf-modules/B/main.tf, change source to absolute path of C

Neither of these is great the first one forces tf-modules to be reachable same way on every system; second one forces B and C to be located in /tf-modules on every system.

RokasDevelopment commented 3 years ago

Terraform is great but I'm surprised this is still not fixed

Running v0.13.3

ronnathaniel commented 3 years ago

+1 getting this on v0.13.5

CaseyLabs commented 3 years ago

Just got bit by this... is there a solution that doesn't involve symlinks or changing the source path of the module? This is a roadblocker for us migrating from an earlier 0.12.x release to 0.13

99 commented 3 years ago

:( same issue on 0.13 and symlinks workaround is not an option

schollii commented 3 years ago

My previous comment gives 2 other ways to get around this, neither of them feasible?

RokasDevelopment commented 3 years ago

My previous comment gives 2 other ways to get around this, neither of them feasible?

I am not sure if I implemented your suggestion correctly, but the first way did not work for me.

pselle commented 3 years ago

Taking a look at this today, I found a few suspicious points to look into.

For the first, there's the symlink issue. ~As some commenters here have pointed out (https://github.com/hashicorp/terraform/issues/23333#issuecomment-671721991) this is likely an OSX-specific issue~ [Edited that this is not the case! So this theory may be completely off] Additionally, the culprit here is likely to be Go, which is the one calling lstat in its evaluation of symlinks. This could perhaps be resolved in Terraform by some investigatory work before directly calling EvalSymLinks in Terraform.

Secondly, when a module is installed, if the module that is installed includes a module reference upwards (../), that relative module is not actually installed. Here, I suspect a solution may lie in updates to go-getter, which is installing that module and thus a possible culprit for not resolving an upwards-relative path for the installed module -- it could also be how Terraform is configuring its go-getter client.

I am sorry that this is not a message informing of a solution, but I wanted to share my notes from an initial dive into this issue.

CaseyLabs commented 3 years ago

@pselle - Hi Pam, just to clarify: I ran into this bug on Ubuntu 20.04 as well (running under Windows 10/WSL2)... so the bug is not restricted to OSX-only ;-)

billyfoss commented 3 years ago

https://github.com/hashicorp/terraform/issues/23333#issuecomment-685704223 references a solution and I believe https://www.terraform.io/docs/modules/sources.html#modules-in-package-sub-directories explains why it works.

This test case is trying to reference a package of modules without telling Terraform where the root of that package can be found. Even though these are not coming from an external source, they still are processed as a package. By using the //, we can let Terraform know to include everything below // in any processing. Without the //, Terraform will only include the lowest directory.
I've forked and patched the original test case and was able to get terraform init to run using v0.12.29 https://github.com/billyfoss/terraform-test-case/commit/cf86c026ea05f5baa873c1775a9200ef0b6093c9

pselle commented 3 years ago

@billyfoss Thanks for adding the link here -- I think that documentation says, to some degree, this is expected behavior, but nothing in the Terraform error helps users update their paths to a working state! Thanks for your addition.

xocasdashdash commented 3 years ago

@billyfoss Thanks for updating it.

@pselle Probably an improved error message saying that a "//" was not found is a good solution

ehillhd commented 3 years ago

I ran into this issue when trying to run terraform init. I would get the below. I remembered I had a generated directory that I had deleted earlier and after creating the generated folder again, I was able to init successfully. Not sure if it's related to this bug but perhaps it might help for others who run into the same thing.

terraform init
Initializing modules...
- generated in 

Error: Unreadable module directory

Unable to evaluate directory symlink: lstat generated: no such file or
directory

Error: Failed to read module directory

Module directory  does not exist or cannot be read.

Error: Unreadable module directory

Unable to evaluate directory symlink: lstat generated: no such file or
directory

Error: Failed to read module directory

Module directory  does not exist or cannot be read.
leriksen commented 3 years ago

For the hashicorp support engineers, I posted a video detailing this issue at https://www.youtube.com/watch?v=25VMiAF5UUM

See my repo with helpful tags at https://github.com/leriksen/tf_rel_path

In summary, this is occurring on our companies TFE, and Hashicorp TFC for v0.12+, but does not occur when using a local state file.

So there may be a clue that the code used in terraform for TFE/TFC is different to what is in this repo. Just a theory.

I am going to investigate the '//' fix next. [edit - it didnt help]

The doco on using local paths is at https://www.terraform.io/docs/modules/sources.html#local-paths

leriksen commented 3 years ago

One other thing to note, perhaps inspired by the comment from pselle a few days ago

In TF11, after running init for my repo with the vtf11 tag checked out, the .terraform file has this structure

infra|1217c9d ⇒ tree .terraform
.terraform
├── modules
│   ├── d6448bb967a955f75f96e0a95539f67e -> /Users/leif/code/src/github.com/leriksen/tf_rel_path/modules/static_data     
│   └── modules.json
├── plugins
│   └── darwin_amd64
│       ├── lock.json
│       └── terraform-provider-null_v2.1.2_x4
└── terraform.tfstate

So you can see TF11 has installed the module, using a symlink

But in TF12, after we run the init, we have this structure

infra|ccd2377 ⇒ tree .terraform
.terraform
├── modules
│   └── modules.json
├── plugins
│   └── darwin_amd64
│       ├── lock.json
│       └── terraform-provider-null_v3.0.0_x5
└── terraform.tfstate

And hence later on when we plan or apply - no module.....

(I had a comment about wild symlinks, but it was completely off point)

a30001539 commented 3 years ago

OK so I have a fix (more a work-around) but I don't like it

If you set the Terraform Working Directory in your TFE/C, you can get the tf client to send more code to the remote, and resolve the rel path issue.

For example, in my sample repo, with the relative path in use for the TF v0.13 client, with the default wd in our TFC settings for the workspace, we fail.

But if I set the wd to /infra/ then we see this difference in the plan/apply output

Preparing the remote apply...

The remote workspace is configured to work with configuration at
    /infra/ relative to the target repository.

Terraform will upload the contents of the following directory,
excluding files or directories as defined by a .terraformignore file
at /Users/leif/code/src/github.com/leriksen/tf_rel_path/.terraformignore (if it is present),
in order to capture the filesystem context the remote workspace expects:
    /Users/leif/code/src/github.com/leriksen/tf_rel_path

So now everything above the infra dir goes to the remote, including our modules dir.

And the plan/apply succeeds - <voice mode='borat'>Great Success !!</voice>

But I don't like it because, if the message is literal, everything in that root goes. That could be a lot more than is really necessary.

I'm going to probe with some local-exec dumps on the TFC filesystem that gets created, to see what gets sent.

So I think the magic spell is to specify the path _back` from your infra code to where the common modules root is. Please let me know your thoughts.

schollii commented 3 years ago

So I think the magic spell is to specify the path _back` from your infra code to where the common modules root is. Please let me know your thoughts.

Is this the same workaround as https://github.com/hashicorp/terraform/issues/23333#issuecomment-693789060?

a30001539 commented 3 years ago

@schollii - I don't think so - my work arounds are specific to using TFE/TFC.

Your referenced comment has part of it with the relative path, but I use relative paths everywhere, no absolute paths at all.

As I said in this comment the issue isn't occurring on local backend for my sample repo in either TF12 or TF13. I haven't tried with S3 or other backends.

I implemented this work-around on two TFE-backed projects for $work today, with the Terraform Working Directory setting for each workspace updated to backtrack to the common root, and both worked fine. I use OSX, and my Windows-centric colleagues confirmed it worked fine for them too.

There is a request in the issue tracker for this repo to add the working directory entry directly to the terraform block, not sure how that is progressing. Interestingly that request predates this issue.

leriksen commented 3 years ago

OK last update. My theory on "unreferenced code" beig uploaded is confirmed. I probed a TFC workspace with this code, which has two modules, one referenced via a relative path, and one not. Both modules directories are upload, which is neither efficient or appropriate.

Probe is a local exec on TFC

provisioner "local-exec" {
  command = "ls -al /terraform/modules"
}

The output shows both modules

null_resource.bump: Creating...
null_resource.bump: Provisioning with 'local-exec'...
null_resource.bump (local-exec): Executing: ["/bin/sh" "-c" "pwd && ls -al /terraform/modules"]
null_resource.bump (local-exec): /terraform/infra
null_resource.bump (local-exec): total 16
null_resource.bump (local-exec): drwxr-xr-x 4 terraform terraform 4096 Nov 24 13:12 .
null_resource.bump (local-exec): drwxr-xr-x 5 terraform root      4096 Nov 24 13:12 ..
null_resource.bump (local-exec): drwxr-xr-x 2 terraform terraform 4096 Nov 24 13:12 no_reference
null_resource.bump (local-exec): drwxr-xr-x 2 terraform terraform 4096 Nov 24 13:12 static_data
null_resource.bump: Creation complete after 0s [id=7713501817599119546]

I think this should be a separate issue, I mention it here just for information.

a30001539 commented 3 years ago

Quick update - for my comment above on more than necessary going.

Judicious use of the .terraformignore file fixes that....