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: Check planfile and put it in the end of an arguments list #1740
fix: Check planfile and put it in the end of an arguments list #1740
Conversation
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.
👍
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 for your contribution! I had two comments regarding the implementation. Once those are addressed, I can kick off a build.
That said, this implementation (where you need to introspect if the arg is a plan file) makes me wonder if there should actually be a dedicated plan_file
configuration in terragrunt.hcl
or CLI (--terragrunt-plan-file
).
options/options.go
Outdated
for _, arg := range argsToInsert { | ||
if checkIfPlanfile(arg) { | ||
planFile = arg | ||
} else { | ||
filteredArgs = append(filteredArgs, arg) | ||
} | ||
} |
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.
Wouldn't this also extract out var files?
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.
@yorinasub17 , var files args are passed in -var-file=filename
form so util.IsFile()
shouldn't return true
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.
Also var files can't be specified with planfile
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.
I've added tests which checks argument lists order
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.
You can pass in var files as terragrunt -var-file filename
too, which I believe would fall into this case, right?
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.
Yes, good catch. Added special check for .tfplan
extension
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.
Updates LGTM! Will kick off a build now.
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.
Regression test passed, so will merge this in. Thanks again for your contribution!
@yorinasub17 thank you for help! Will be happy to join if there is some roadmap |
This issue should fix #1271
This MR fixes args order when planfile is specified in extra_args for apply