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

Fix. Correct how CORS rules are handled. #8096

Merged
merged 1 commit into from
Aug 12, 2016

Conversation

kwilczynski
Copy link
Contributor

This commit fixes an issue where CORS rules would not be read and thus refreshed
correctly should there be a change introduced externally e.g. CORS configuration
was edited outside of Terraform.

Signed-off-by: Krzysztof Wilczynski krzysztof.wilczynski@linux.com

@kwilczynski
Copy link
Contributor Author

Test is passing:

$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSS3Bucket_Cors'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/08/10 18:30:34 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSS3Bucket_Cors -timeout 120m
=== RUN   TestAccAWSS3Bucket_Cors
--- PASS: TestAccAWSS3Bucket_Cors (50.99s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    51.008s

@kwilczynski kwilczynski changed the title Fix. Correct how CORS rules are handled. [WIP] Fix. Correct how CORS rules are handled. Aug 10, 2016
@kwilczynski
Copy link
Contributor Author

@stack72 this corrects the behaviour, but now I need to also update the test, so that it can actually trigger the path where ReadFunc would be used. At the moment test only hit the UpdateFunc route, which is why nobody noticed that there is an issue.

This commit fixes an issue where CORS rules would not be read and thus refreshed
correctly should there be a change introduced externally e.g. CORS configuration
was edited outside of Terraform.

Signed-off-by: Krzysztof Wilczynski <krzysztof.wilczynski@linux.com>
@kwilczynski kwilczynski force-pushed the fix/cors_rule-aws_s3_bucket branch from 2915709 to 639e0ad Compare August 11, 2016 10:31
@kwilczynski kwilczynski changed the title [WIP] Fix. Correct how CORS rules are handled. Fix. Correct how CORS rules are handled. Aug 11, 2016
@stack72
Copy link
Contributor

stack72 commented Aug 12, 2016

Thanks @kwilczynski

Really nice spot that we were only setting the data back to state when we actually errored! Also great spot that we had to flatten []string to state as well

% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSS3Bucket_'                                                                                                             2 ↵
==> Checking that code complies with gofmt requirements...
/Users/stacko/Code/go/bin/stringer
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/08/12 13:06:30 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSS3Bucket_ -timeout 120m
=== RUN   TestAccAWSS3Bucket_Notification
--- PASS: TestAccAWSS3Bucket_Notification (244.84s)
=== RUN   TestAccAWSS3Bucket_NotificationWithoutFilter
--- PASS: TestAccAWSS3Bucket_NotificationWithoutFilter (89.28s)
=== RUN   TestAccAWSS3Bucket_basic
--- PASS: TestAccAWSS3Bucket_basic (66.08s)
=== RUN   TestAccAWSS3Bucket_acceleration
--- PASS: TestAccAWSS3Bucket_acceleration (142.33s)
=== RUN   TestAccAWSS3Bucket_RequestPayer
--- PASS: TestAccAWSS3Bucket_RequestPayer (141.49s)
=== RUN   TestAccAWSS3Bucket_Policy
--- PASS: TestAccAWSS3Bucket_Policy (168.67s)
=== RUN   TestAccAWSS3Bucket_UpdateAcl
--- PASS: TestAccAWSS3Bucket_UpdateAcl (140.81s)
=== RUN   TestAccAWSS3Bucket_Website_Simple
--- PASS: TestAccAWSS3Bucket_Website_Simple (211.71s)
=== RUN   TestAccAWSS3Bucket_WebsiteRedirect
--- PASS: TestAccAWSS3Bucket_WebsiteRedirect (234.69s)
=== RUN   TestAccAWSS3Bucket_WebsiteRoutingRules
--- PASS: TestAccAWSS3Bucket_WebsiteRoutingRules (143.84s)
=== RUN   TestAccAWSS3Bucket_shouldFailNotFound
--- PASS: TestAccAWSS3Bucket_shouldFailNotFound (48.80s)
=== RUN   TestAccAWSS3Bucket_Versioning
--- PASS: TestAccAWSS3Bucket_Versioning (196.87s)
=== RUN   TestAccAWSS3Bucket_Cors
--- PASS: TestAccAWSS3Bucket_Cors (158.54s)
=== RUN   TestAccAWSS3Bucket_Logging
--- PASS: TestAccAWSS3Bucket_Logging (126.91s)
=== RUN   TestAccAWSS3Bucket_Lifecycle
--- PASS: TestAccAWSS3Bucket_Lifecycle (148.04s)
PASS
ok github.com/hashicorp/terraform/builtin/providers/aws 2254.833s

@stack72 stack72 merged commit 168d212 into hashicorp:master Aug 12, 2016
@ghost
Copy link

ghost commented Apr 23, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants