Skip to content

cdn: Handle certificate name updates.#579

Merged
andrewsomething merged 2 commits intomainfrom
asb/issues/578
Feb 5, 2021
Merged

cdn: Handle certificate name updates.#579
andrewsomething merged 2 commits intomainfrom
asb/issues/578

Conversation

@andrewsomething
Copy link
Copy Markdown
Member

@andrewsomething andrewsomething commented Feb 5, 2021

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
Copy Markdown
Member Author

andrewsomething commented Feb 5, 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.

bentranter
bentranter approved these changes Feb 5, 2021
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown

liarco commented Feb 6, 2021 •
edited
Loading

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
@andrewsomething
cdn: Handle certificate name updates. (#579)
099fae8 
* 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

Reviewers

@bentranter bentranter bentranter approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@andrewsomething @liarco @bentranter