Skip to content
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

Add test to show random struct key sorting in backend config #176

Closed

Conversation

joe-niland
Copy link
Sponsor Member

@joe-niland joe-niland commented Jul 7, 2022

what

  • adding a (rough) test that demonstrates randomly changing terraform.backend.s3 key order in backend config file generation

why

  • We’ve noticed that backend.tf.json constantly changes the order of the keys within the s3 element. From what I can tell it’s due to go iterating over struct keys using a random order.

references

  • N/A

test output

❯ make testacc TEST=github.com/cloudposse/atmos/pkg/utils
go get
go test github.com/cloudposse/atmos/pkg/utils -v  -timeout 2m
=== RUN   TestBackendConfig
    json_utils_test.go:65: 
                Error Trace:    /Users/joe/git-proj/cloudposse/atmos/pkg/utils/json_utils_test.go:65
                Error:          Not equal: 
                                expected: "{\"terraform\":{\"backend\":{\"s3\":{\"encrypt\":true,\"key\":\"terraform.tfstate\",\"region\":\"us-east-1\",\"role_arn\":null,\"workspace_key_prefix\":\"app\",\"acl\":\"bucket-owner-full-control\",\"bucket\":\"sts-gbl-tfstate-backend\",\"dynamodb_table\":\"sts-gbl-tfstate-backend-lock\"}}}}"
                                actual  : "{\"terraform\":{\"backend\":{\"s3\":{\"dynamodb_table\":\"cp-ue2-root-tfstate-lock\",\"profile\":\"cp-gb2-root-tfstate\",\"role_arn\":null,\"workspace_key_prefix\":\"test-test-component\",\"region\":\"us-east-2\",\"acl\":\"bucket-owner-full-control\",\"bucket\":\"cp-ue2-root-tfstate\",\"encrypt\":true,\"key\":\"terraform.tfstate\"}}}}"
                            
                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -1 +1 @@
                                -{"terraform":{"backend":{"s3":{"encrypt":true,"key":"terraform.tfstate","region":"us-east-1","role_arn":null,"workspace_key_prefix":"app","acl":"bucket-owner-full-control","bucket":"sts-gbl-tfstate-backend","dynamodb_table":"sts-gbl-tfstate-backend-lock"}}}}
                                +{"terraform":{"backend":{"s3":{"dynamodb_table":"cp-ue2-root-tfstate-lock","profile":"cp-gb2-root-tfstate","role_arn":null,"workspace_key_prefix":"test-test-component","region":"us-east-2","acl":"bucket-owner-full-control","bucket":"cp-ue2-root-tfstate","encrypt":true,"key":"terraform.tfstate"}}}}
                Test:           TestBackendConfig
--- FAIL: TestBackendConfig (0.06s)
FAIL
FAIL    github.com/cloudposse/atmos/pkg/utils   0.404s
FAIL
make: *** [testacc] Error 1

❯ make testacc TEST=github.com/cloudposse/atmos/pkg/utils
go get
go test github.com/cloudposse/atmos/pkg/utils -v  -timeout 2m
=== RUN   TestBackendConfig
    json_utils_test.go:65: 
                Error Trace:    /Users/joe/git-proj/cloudposse/atmos/pkg/utils/json_utils_test.go:65
                Error:          Not equal: 
                                expected: "{\"terraform\":{\"backend\":{\"s3\":{\"encrypt\":true,\"key\":\"terraform.tfstate\",\"region\":\"us-east-1\",\"role_arn\":null,\"workspace_key_prefix\":\"app\",\"acl\":\"bucket-owner-full-control\",\"bucket\":\"sts-gbl-tfstate-backend\",\"dynamodb_table\":\"sts-gbl-tfstate-backend-lock\"}}}}"
                                actual  : "{\"terraform\":{\"backend\":{\"s3\":{\"workspace_key_prefix\":\"test-test-component\",\"encrypt\":true,\"role_arn\":null,\"dynamodb_table\":\"cp-ue2-root-tfstate-lock\",\"key\":\"terraform.tfstate\",\"profile\":\"cp-gb2-root-tfstate\",\"region\":\"us-east-2\",\"acl\":\"bucket-owner-full-control\",\"bucket\":\"cp-ue2-root-tfstate\"}}}}"
                            
                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -1 +1 @@
                                -{"terraform":{"backend":{"s3":{"encrypt":true,"key":"terraform.tfstate","region":"us-east-1","role_arn":null,"workspace_key_prefix":"app","acl":"bucket-owner-full-control","bucket":"sts-gbl-tfstate-backend","dynamodb_table":"sts-gbl-tfstate-backend-lock"}}}}
                                +{"terraform":{"backend":{"s3":{"workspace_key_prefix":"test-test-component","encrypt":true,"role_arn":null,"dynamodb_table":"cp-ue2-root-tfstate-lock","key":"terraform.tfstate","profile":"cp-gb2-root-tfstate","region":"us-east-2","acl":"bucket-owner-full-control","bucket":"cp-ue2-root-tfstate"}}}}
                Test:           TestBackendConfig
--- FAIL: TestBackendConfig (0.04s)
FAIL
FAIL    github.com/cloudposse/atmos/pkg/utils   0.241s
FAIL
make: *** [testacc] Error 1

@nitrocode
Copy link
Member

nitrocode commented Jul 8, 2022

We discussed this in the sweetops slack community (thread)

Here are the current options to solve this issue for you

  1. We make backend.tf.json generation deterministic in atmos
  2. Workaround: You can add backend.tf.json to a .gitignore and allow atmos to do its thing (provided no one needs to run terraform without atmos)
  3. Workaround: You can hard code the backend for all terraform modules and disable atmos from generating the backend.tf.json

For option 1, to make this deterministic, we have some more options

  1. Use a library (see thread encoding/json: no way to preserve the order of map keys golang/go#27179)
  2. Use a custom json Marshaller
  3. Sort the keys manually and then write them to a file

@joe-niland
Copy link
Sponsor Member Author

@nitrocode thanks for the great summary! Closing.

@joe-niland joe-niland closed this Jul 17, 2022
@nitrocode
Copy link
Member

It would be nice to keep this open or at least create an issue to track this so we can fix it in the future

@joe-niland joe-niland reopened this Jul 17, 2022
@joe-niland
Copy link
Sponsor Member Author

@nitrocode that's true. I'll rewrite it to describe the issue a bit better.

@aknysh
Copy link
Member

aknysh commented Sep 7, 2022

@joe-niland this was fixed in the last merged PR, atmos release 1.5.0

https://github.com/cloudposse/atmos/pull/189/files#diff-873ae70d0bd7f36db36e19041297b6a195f93676374b57ac1687eeba4b043cffR24

I'll close this PR for now, please reopen if any issues

@aknysh aknysh closed this Sep 7, 2022
@joe-niland
Copy link
Sponsor Member Author

@joe-niland this was fixed in the last merged PR, atmos release 1.5.0

Thanks for letting me know @aknysh ! Glad you found a solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants