-
Notifications
You must be signed in to change notification settings - Fork 185
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
Fix remote repo panic #86
Conversation
Also, I removed the ecs-cluster example as I don't think the submodule was actually pointing to anything. If u want to re-add it, definitely do. Also, I added a nested-module example just for testing. It actually points back at this repo and uses the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
It's interesting how this doesn't work on remote modules since rover uses the terraform plan output. The init command downloads remote modules and stores it in the .terraform dir, when you run a plan or an apply, it should use module in .terraform.
Even though this looks on a spot check, gonna verify the above before merging. (Aiming for Wed)
If I can't get to it by Wed, I'll just merge this as a stop gap 😄
Oh yeah @im2nguyen , line numbers and file names require To access line numbers and file names of nested modules, it essentially calls I'll see if there's a way to parse the module call and access the module in the .terraform folder. If so, I'll commit it to this PR :). |
Okay @im2nguyen , it feels a bit hackish but these commits directly parse the I also removed the -tfConfigExists flag, since there's no longer any use for it. If the modules aren't available locally, rover just prints either
Let me know if this seems like the right approach! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good! I ran into the following error while testing on the nested-test directory, but I assumed this was bc the main branch still references eks-cluster
. It worked when I tested using another remote module though! 😄
$ ./rover -workingDir example/nested-test/
2022/01/24 10:33:45 Starting Rover...
2022/01/24 10:33:45 Initializing Terraform...
2022/01/24 10:33:55 Unable to parse Plan: Unable to initialize Terraform Plan: exit status 1
Error: Failed to download module
Could not download module "remote_module" (nested-module/main.tf:1) source
code from "git::https://github.com/im2nguyen/rover.git": error downloading
'https://github.com/im2nguyen/rover.git': /usr/local/bin/git exited with 128:
fatal: No url found for submodule path 'example/eks-cluster' in .gitmodules
Error: Failed to download module
Could not download module "remote_module" (nested-module/main.tf:1) source
code from "git::https://github.com/im2nguyen/rover.git": error downloading
'https://github.com/im2nguyen/rover.git': /usr/local/bin/git exited with 128:
fatal: No url found for submodule path 'example/eks-cluster' in .gitmodules
@@ -0,0 +1,4 @@ | |||
module "sub_module" { | |||
source = "./nested-module" | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
source = "git::https://github.com/im2nguyen/rover.git//example/random-test/random-name" | ||
|
||
max_length = "3" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @im2nguyen, this should resolve #84 . It just checks to make sure the module is not at a remote url before trying to load the module (for line number and file info).