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 import support for ssl certificate, http/s proxy and url map #678

Merged
merged 1 commit into from
Nov 9, 2017

Conversation

krotkiewicz
Copy link
Contributor

$ TF_ACC=1 go test ./google -v -run TestAccComputeUrlMap -timeout 120m
=== RUN   TestAccComputeUrlMap_import
=== RUN   TestAccComputeUrlMap_basic
=== RUN   TestAccComputeUrlMap_update_path_matcher
=== RUN   TestAccComputeUrlMap_advanced
=== RUN   TestAccComputeUrlMap_noPathRulesWithUpdate
--- PASS: TestAccComputeUrlMap_basic (69.27s)
--- PASS: TestAccComputeUrlMap_import (72.18s)
--- PASS: TestAccComputeUrlMap_noPathRulesWithUpdate (83.70s)
--- PASS: TestAccComputeUrlMap_update_path_matcher (85.37s)
--- PASS: TestAccComputeUrlMap_advanced (143.10s)
PASS
ok  	github.com/terraform-providers/terraform-provider-google/google	143.133s

$ TF_ACC=1 go test ./google -v -run TestAccComputeTargetHttp
=== RUN   TestAccComputeTargetHttpProxy_import
=== RUN   TestAccComputeTargetHttpsProxy_import
=== RUN   TestAccComputeTargetHttpProxy_basic
=== RUN   TestAccComputeTargetHttpProxy_update
=== RUN   TestAccComputeTargetHttpsProxy_basic
=== RUN   TestAccComputeTargetHttpsProxy_update
=== RUN   TestAccComputeTargetHttpsProxy_invalidCertificate
--- PASS: TestAccComputeTargetHttpsProxy_invalidCertificate (0.04s)
--- PASS: TestAccComputeTargetHttpProxy_basic (93.05s)
--- PASS: TestAccComputeTargetHttpsProxy_basic (93.94s)
--- PASS: TestAccComputeTargetHttpsProxy_import (95.11s)
--- PASS: TestAccComputeTargetHttpProxy_import (104.19s)
--- PASS: TestAccComputeTargetHttpProxy_update (113.31s)
--- PASS: TestAccComputeTargetHttpsProxy_update (120.06s)
PASS
ok  	github.com/terraform-providers/terraform-provider-google/google	120.100s

$ TF_ACC=1 go test ./google -v -run TestAccComputeSslCertificate
=== RUN   TestAccComputeSslCertificate_import
=== RUN   TestAccComputeSslCertificate_basic
=== RUN   TestAccComputeSslCertificate_no_name
=== RUN   TestAccComputeSslCertificate_name_prefix
--- PASS: TestAccComputeSslCertificate_name_prefix (23.85s)
--- PASS: TestAccComputeSslCertificate_import (23.89s)
--- PASS: TestAccComputeSslCertificate_basic (25.72s)
--- PASS: TestAccComputeSslCertificate_no_name (30.87s)
PASS
ok  	github.com/terraform-providers/terraform-provider-google/google	30.912s

@krotkiewicz
Copy link
Contributor Author

@danawillow would you mind reviewing this PR ? should I create corresponding issues ?

@rosbo
Copy link
Contributor

rosbo commented Nov 6, 2017

No need to create corresponding issues. We already have one for adding import functionality to compute resources #182

@rosbo rosbo self-assigned this Nov 6, 2017
@krotkiewicz
Copy link
Contributor Author

Hello @rosbo
Is there anything I can do to get this PR approved faster ?

@rosbo rosbo self-requested a review November 8, 2017 17:52

hostRuleMap := make(map[string]*compute.HostRule)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change. It changes the behavior for host_rule, path_matcher and test.

In the old behavior, if you had some entries created from outside Terraform, it would not create a diff. Now, it would show a diff. For proper import support however, I know this is required.

To be honest, it is probably a good thing. Better to manage everything in Terraform. However, since it is changing the behavior, we will need to wait longer before we release it. We do frequent release of non-breaking changes. And longer cycle (we wait until we have a few breaking changes) before doing a release with breaking changes. We do this to avoid the pain to our users of having to deal constantly with breaking changes requiring them to update their config.

The other 3 resources are good to go. Can you put the url map in a separate PR? I will merge the other 3 resources and then we will merge the url map separately when we are ready to do a breaking change release.

Thank you

@rosbo
Copy link
Contributor

rosbo commented Nov 8, 2017

Thank you Konrad for adding import support for all these resources! :)

@krotkiewicz krotkiewicz force-pushed the more_imports branch 2 times, most recently from 5f39eb8 to 0b7ece9 Compare November 9, 2017 08:38
@krotkiewicz
Copy link
Contributor Author

krotkiewicz commented Nov 9, 2017

@rosbo I removed this breaking change but the importer can't import these 3 properties - so I ignored them in the test.

This way we have half of the import done and we are backward compatible.

The url map part is here #710, I will rebase it after merging this PR.

Copy link
Contributor

@rosbo rosbo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@rosbo rosbo merged commit a7c8d04 into hashicorp:master Nov 9, 2017
chrisst pushed a commit to chrisst/terraform-provider-google that referenced this pull request Nov 9, 2018
@ghost
Copy link

ghost commented Mar 30, 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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

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

Successfully merging this pull request may close these issues.

2 participants