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

cdn: Handle certificate name updates. #579

Merged
merged 2 commits into from Feb 5, 2021
Merged

Conversation

andrewsomething
Copy link
Member

This resolves a bug introduced in #500. We made changes to use certificate names as primary identifier instead of IDs. This included the CDN resource, but I missed the update flow for CDNs. We did not have an acceptance test excercising using CDNs with custom domains/certificates.

The new test fails before the code changes are appiled:

$ make testacc TESTARGS='-run=TestAccDigitalOceanCDN_CustomDomain'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run=TestAccDigitalOceanCDN_CustomDomain -timeout 120m
?       github.com/digitalocean/terraform-provider-digitalocean [no test files]
=== RUN   TestAccDigitalOceanCDN_CustomDomain
=== PAUSE TestAccDigitalOceanCDN_CustomDomain
=== CONT  TestAccDigitalOceanCDN_CustomDomain
    resource_digitalocean_cdn_test.go:114: Step 2/2 error: Error running apply: 
        Error: Error deleting Certificate: DELETE https://api.digitalocean.com/v2/certificates/a25f53d6-fdc7-4230-bf25-c6266d48d81a: 422 (request "559bc985-3360-4c4d-a124-71ba0bd83538") This certificate is being used by an active Load Balancer. You must make sure no Load Balancer is using it before deleting.
        
        
--- FAIL: TestAccDigitalOceanCDN_CustomDomain (54.61s)
FAIL
FAIL    github.com/digitalocean/terraform-provider-digitalocean/digitalocean    54.623s
testing: warning: no tests to run
PASS
ok      github.com/digitalocean/terraform-provider-digitalocean/internal/datalist       (cached) [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/digitalocean/terraform-provider-digitalocean/internal/mutexkv        (cached) [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/digitalocean/terraform-provider-digitalocean/internal/setutil        (cached) [no tests to run]
FAIL
make: *** [GNUmakefile:16: testacc] Error 1

When the changes are applied, the test now passes:

$ make testacc TESTARGS='-run=TestAccDigitalOceanCDN_CustomDomain'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run=TestAccDigitalOceanCDN_CustomDomain -timeout 120m
?       github.com/digitalocean/terraform-provider-digitalocean [no test files]
=== RUN   TestAccDigitalOceanCDN_CustomDomain
=== PAUSE TestAccDigitalOceanCDN_CustomDomain
=== CONT  TestAccDigitalOceanCDN_CustomDomain
--- PASS: TestAccDigitalOceanCDN_CustomDomain (95.23s)
PASS
ok      github.com/digitalocean/terraform-provider-digitalocean/digitalocean    95.238s
testing: warning: no tests to run
PASS
ok      github.com/digitalocean/terraform-provider-digitalocean/internal/datalist       (cached) [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/digitalocean/terraform-provider-digitalocean/internal/mutexkv        (cached) [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/digitalocean/terraform-provider-digitalocean/internal/setutil        (cached) [no tests to run]

@andrewsomething
Copy link
Member Author

This should fix #578 Though I was not able to recreate the exact same error. I'd like to get a bug fix release out for them to test.

Copy link
Member

@bentranter bentranter left a comment

Choose a reason for hiding this comment

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

LGTM!

@andrewsomething andrewsomething merged commit 81ca350 into main Feb 5, 2021
@andrewsomething andrewsomething deleted the asb/issues/578 branch February 5, 2021 20:18
@liarco
Copy link

liarco commented Feb 6, 2021

This should fix #578 Though I was not able to recreate the exact same error. I'd like to get a bug fix release out for them to test.

@andrewsomething I can confirm our bug is fixed with this PR. Thank you very much.

andrewsomething added a commit that referenced this pull request Apr 26, 2021
* cdn: Add testcase for creating and updating with custom domain/cert.

* cdn: Handle certificate name updates.
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