Fixes issues with AWSAssumeRole in Blocks for Terraform being passed in #1720
Conversation
| func logCommandFail(exitCode int, err error) { | ||
|
|
||
| _, filename, _, ok := runtime.Caller(1); | ||
| if ok { | ||
| executor := strings.TrimSuffix(path.Base(filename), path.Ext(filename)) | ||
| log.Printf("Command failed in %v with exit code %v and error %v", executor, exitCode, err) | ||
| } else { | ||
| log.Printf("Command failed in unknown executor with exit code %v and error %v", exitCode, err) | ||
| } | ||
| } |
There was a problem hiding this comment.
thanks so much for this improvement, I remember terragrunt jobs always had obscure error messages especially when they failed
| // Terragrunt will cause a backend configuration problem if backend-config options are passed and envs of the same key are passed. | ||
| // which will trigger a request to init with --reconfigure, so do not use backend-config for terragrunt | ||
| if job.Terragrunt != true { |
There was a problem hiding this comment.
today I learned something new, so with terragrunt does it pass those options to terraform based on terragrunt.hcl configuration or what is the source of the duplicates?
There was a problem hiding this comment.
From the docs I read it should not happen but in practice I tested it out and it consistently happened, if I set aws credentials in the my environment and then also pass in backend-config options for credentials causes that request on state change when it runs the plan.
The init would work fine with both backend-config and environment variables.
When the plan would run, since backend-config variables were passed only into the init step it would detect a remote change even though the backend file was the same (must add some hash or something of the complete config in terragrunt / terraform somewhere)
So the option was to make it pass backend-config to all steps or just exclude it. I chose to exclude it path, as it does not seem to impact terraform. backend-config is not recommended on the TF site, I figured it was best at least for TG to just ignore their use.
I did not go much deeper than that, but I know the above does work.
libs/scheduler/aws.go
Outdated
| } else { | ||
| job.StateEnvVars, err = populateKeys(job.StateEnvVars, *job.StateEnvProvider) | ||
| if err != nil { | ||
| log.Printf("Failed to get keys from role (StateEnvProvider): %v", err) | ||
| return fmt.Errorf("Failed to get (state) keys from role: %v", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
So this is not a challenge and I'm probably wrong because I haven't looked at this code in a while but from memory I remmber that the reason for using "backend configuration" passed as arguments rather than env variables was to tell terraform to pull S3 state from an account different from the account where it is creating resources.
From my basic tests it is not possible to achieve this with env variables during the init phase. SO, I wonder if this entire else block is needed at all. Because if terragrunt has its own way to pass the same config then maybe we ignore the else entirely.
I'm pretty sure I missed something but very useful to discuss this on here for historical context of the code
There was a problem hiding this comment.
The StateEnvVars only has 3 values from what I can tell: access key, secret and token. There would not be anything specific in there related to which account it sets up. If the backend is specifying a different account then the state role that gets used would have access to that account via trust policy and get the correct credentials to pass into the init environment command. There may be some where else that is tying that provider to an account I am not sure.
The environment variables are passed into the subshell command later so command / state env vars can still be different if using two accounts one for state management and one for actual resources. Thats not how we do it but I can see why some people would set it up that way if they uses remote states across many projects.
|
@motatoes I am not sure why that test is failing it looks related to OpenTofu are you able to take a look and let me know if this is one that needs addressed before it get's rolled in? |
| }; | ||
|
|
||
| // allow blocks to pass in roles that can be assummed by aws | ||
| tgParsingConfig.AwsRoleToAssume = b.AwsRoleToAssume |
There was a problem hiding this comment.
sorry another nit, why not just pass it in the struct above directly? :D
Fixes #1714