Fixes for multienv step comment

[hatmarch]

As part of a recent Canva hackathon, we made a small PoC to investigate how Atlantis might improve our internal terraform applications (see also here)

In working with the custom workflow, we found that there are two issues with the multienv step:

• It doesn't correctly handle environment variable values that themselves contain "=" (such as AWS session tokens that look like this: FwoGZXIvYXdzEBEaDGoKFI7Oqa1Q9zrmySKsAQoLPoXII+HGELWjXAnNeD/STJ62afxU9tp7HKwOWoNV0KBwD1gE6ktmagm7/UPGe+oqeL5OswpB2UW1WY5t1kh/ImVYfhnwAQZhhD2Gydqem2XUs92JDxgK1ecurlwPzzfQs0HWMmSnc5V3hqcTC8DiiclFH5aPxWd0CM2zJjKmab0MQ4WfEv/Em08NwDQAm5vNpe5KFAO0pJiMqYC1HXo6iRyRwClzZALCb5woutrllgYyLeaSetw+lmd0c8gyVLuUEJTXFf3dKnuTpUutmS95vOed6+/2rMm3IF8WRu1B+A==)
• The last environment variable that is given has a trailing "\n" as part of its value

This PR presents a fix for this.

NOTE: I'm open to feedback on this, for instance how I should update the multienv test to add these cases to the test suite

[jamengual]

is this similar to this ? https://github.com/runatlantis/atlantis/pull/2354

[arohter]

#2354 fixes the = logic, but there's still the extra \n bug.

There's also currently another bug, in that an empty response (no env to set) is not allowed. Specifically, looks like misunderstanding of golang's string.Split() impl: https://github.com/runatlantis/atlantis/blob/master/server/core/runtime/multienv_step_runner.go#L21 will never be zero (split will return an array of size 1 , with empty string inside).

[jamengual]

@hatmarch can you fix the conflicts and check @arohter comment?

[lilincmu]

@hatmarch It will be greatly appreciated if you can help us fix the conflict and possibly trim res and compare it against empty string first!

[hatmarch]

@lilincmu (et. al.) Let me know if this is what you had in mind

[comjf]

Hello everyone. I'm also following along with this issue thread. I was originally blocked by the second = as it's a base64 encoded value. Then I updated to the v0.19.8 version and I now multienv doesn't error, but my TF init does. I see the embedded \n in the response:

....bQKei4I=\n" for key X-Amz-Security-Token

So I guess I'm also waiting on this fix.

That being said, please tell me if I'm doing something stupid with this entire workflow. I just really don't want to update thousands of terraform_remote_state to add a role_arn. It makes rolling this out hard as I don't want to break existing functionality for developers running terraform locally (who may not have IAM permission to assume this new role)

Therefore, I've added a custom workflow to inject AWS variables to the shell running atlantis:

workflows:
  legacy:
    plan:
      steps:
        - multienv: printf "AWS_ACCESS_KEY_ID=%s,AWS_SECRET_ACCESS_KEY=%s,AWS_SESSION_TOKEN=%s" $(aws sts assume-role --role-arn arn:aws:iam::11111111111:role/delegated-atlantis-role --role-session-name atlantis-execution --query "Credentials.[AccessKeyId,SecretAccessKey,SessionToken]" --output text)
        - init
        - plan

My hope is that this will allow existing usage of terraform (locally), as well as execution from the atlantis service running in a different AWS account.

[Dilergore]

is there an ETA for this to be released?

[jamengual]

it will probably get merged next week and then we will release a pre-release and then 2 or 3 weeks after a new release.