fix(lib): fix source path generated for local modules

[JonCubed]

This fixes issue #946

[hashicorp-cla]

All committers have signed the CLA.

[jsteinich]

@JonCubed Very much appreciate the fix; however, at least longer term I think that local modules need to be treated like an asset (see #648). The reason being is so that they work when using Terraform Cloud.
Since we haven't actually gotten to putting that in place yet, it may still make sense to use this in the short term though.

[JonCubed]

@jsteinich what is proposed for Assets was what I initial thought would happen with modules, so looks good to me long term. Short term I would appreciate if this could be accepted as use local modules is currently broken.

[javiortizmol]

@jsteinich what is proposed for Assets was what I initial thought would happen with modules, so looks good to me long term. Short term I would appreciate if this could be accepted as use local modules is currently broken.

Agree with you @JonCubed, I tested the TerraformAsset option and it didn't work @jsteinich because in the cdk.tf.json file, the synt command is puting an absolute path instead of a relative path and also you can't set a relative path on the TerraformAsset, it should be absolute.

import os

        tfEc2Module = TerraformAsset(self, "MyEc2Module",
            path = os.path.abspath("./modules/compute")
        )
        my_ec2 = TerraformHclModule(self, "My-ec2", 
            source = tfEc2Module.path,
            variables = {
                "ami": Locals.get("ami").get(config.environment),
                "instance_type": Locals.get("instance_type").get(config.environment),
                "vpc_security_group_ids": Locals.get("vpc_security_group_ids").get(config.environment),
                "subnet_id": Locals.get("vpc").get("public_subnets_ids").get(config.environment),
                "key_name": Locals.get("key_name").get(config.environment),
                "additional_tags": { "Name": f"python-{getWorkspace()}" }                
            }
        )

I tried adding the "./" to the source = f"./{tfEc2Module.path}", but I crashed with the same path validation, that logic is changing the "./" that I added by "../".

╷
│ Error: Module not found
│ 
│ The module address "assets/MyEc2Module" could not be resolved.
│ 
│ If you intended this as a path relative to the current module, use "./assets/MyEc2Module" instead. The
│ "./" prefix indicates that the address is a relative filesystem path.

I reviewed the cdk.tf.json file generated and I found "source": "assets/MyEc2Module" this is an absolute path, it is the reason behind of the error message.

I modified the cdk.tf.json file manually, changing it for "source": "./assets/MyEc2Module", then on the cdktf.out/stacks/myStack folder, I ran the terraform init command and works.

None of the solutions you mention work @jsteinich, why don't you just accept the solution from @JonCubed ?

[jsteinich]

None of the solutions you mention work @jsteinich, why don't you just accept the solution from @JonCubed ?

I'm not sure the exact order of operations, but @DanielMSchmidt also started a PR for doing the "longer term" solution. Given the asset relative directory complications, I certainly agree with merging this approach. If the other PR finishes in time for the next release, great, but if not then the situation is still improved.

@JonCubed apologizes for not responding sooner. I don't have the ability to merge, but I can certainly poke the rest of the team.

[JonCubed]

no worries @jsteinich , I've worked around it by patching my local and docker image.

I've played around with this some more and one thing that might make this fix undesirable is that it only works for the default outputs i.e. cdktf.out/stacks/. In order to make it work for any output folder structure we would need to get the value from the output flag or cdktf.json to determine how far back we need to go. I wasn't sure how I could get this value and if this is a deal breaker or not for the short term.

[DanielMSchmidt]

@JonCubed Could you rebase this?

[JonCubed]

@DanielMSchmidt rebased

[JonCubed]

@jsteinich @DanielMSchmidt I've rebased this again. Any chance we can merge this or co-ordinate so that this can be merged?

[JonCubed]

@DanielMSchmidt thank you

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you've found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.