Allow for s3 backend path style #53
Conversation
bin/tflocal
Outdated
| @@ -220,6 +221,7 @@ def generate_s3_backend_config() -> str: | |||
| "key": "terraform.tfstate", | |||
| "dynamodb_table": "tf-test-state", | |||
| "region": get_region(), | |||
| "use_path_style": "true", | |||
There was a problem hiding this comment.
I'm not sure everyone would be happy if we pin this option here or force them to add it explicitly with false when they don't need it.
The best option probably if we introduce a similar logic like this instead:
Line 136 in 07643c4
(Sidenote: users use tflocal in combination with real s3 backends too, so we must not break those setups either.)
There was a problem hiding this comment.
Agreed, I will work on that!
There was a problem hiding this comment.
I set the default to false and I am reusing the use_s3_path_style method so it should be easier for a user to use different combinations of backend/s3 target
| @@ -60,6 +60,7 @@ terraform { | |||
| bucket = "<bucket>" | |||
| key = "<key>" | |||
| dynamodb_table = "<dynamodb_table>" | |||
| use_path_style = "<use_path_style>" | |||
There was a problem hiding this comment.
This must be added dynamically, see my other comment below why.
There was a problem hiding this comment.
To keep transparency and a similar style, I left it here, but changed the default to false.
| @@ -247,6 +249,9 @@ def generate_s3_backend_config() -> str: | |||
| get_or_create_bucket(configs["bucket"]) | |||
| get_or_create_ddb_table(configs["dynamodb_table"], region=configs["region"]) | |||
| result = TF_S3_BACKEND_CONFIG | |||
| if is_tf_legacy: | |||
There was a problem hiding this comment.
Nice backward compatibility proofing. I believe we should introduce the same logic for the override file too if we address this here. Fancy to add it somewhere here?
Line 136 in 07643c4
There was a problem hiding this comment.
I looked at that, and if I understand properly, you are suggesting to change the configuration of the aws provider from s3_use_path_style supported in provider >4 to s3_force_path_style for providers version <4.
It can be quite the complex problem to define which version of a provider will be used as all modules are looked at by terraform before deciding on the providers version.
Let me know if I am missing something 🤔
…termine if s3 path style is required
Motivation
Using
tflocalas part of a docker-compose configuration requires to use path style for s3 bucket. While the provider configuration is enabling path style for buckets created by terraform, this configuration did not extend to s3 backend. As reported on issue #25 and in the Community Slack, use/force_path_style configuration was not respectedChanges
Set the default when overriding the backend to use path style. This decision came as this is also seems to be what is enabled by default when encountering s3 services.
use_path_stylehas been in use since tf v1.6, prior version usedforce_path_style.There is a bit of a hacky way to switch. Maybe if more deprecation issue arise, a more elegant solution would be required to not turn this into spaghetti 😄
Not In Scope
There are many other attributes not currently supported such as the mentioned
encryptandacl. Those changes might require a bit more testing with LocalStack and Terraform to make sure it stays compatible, so were left out of scopeTesting
No new test was introduced as the backend is already tested with legacy and latest Terraform cli. If I am missing a testing opportunity, I am all ears!