upgrade provider to use terraform-plugin-sdk v2

[tdyas]

This PR upgrades the provider to use v2 of the terraform-plugin-sdk.

This makes the minimum necessary changes to get the provider to work:

• The hashcode.String function was made internal to the SDK. The implementation was copied to a new function: SDKHashString

• terraform.ResourceProvider was removed. Code now uses *schema.Provider directly.

• schema.CustomizeDiffFunc now takes context.Context parameter.

• The SetPartial function was removed. I left in the calls to d.Partial but the partial state usage need to be audited for correctness.

• The Removed attribute for obsolete fields was itself removed. I removed the instance of it in the provider.

Why upgrade to v2? It provides the opportunity to update resources/datasources to get passed context.Context parameters plus provider better diagnostics in error cases among other new features.

@andrewsomething: Could you submit this for a full run of the acceptance tests?

[andrewsomething]

Thanks for taking this on @tdyas! I haven't done a deep review yet, but I wanted to flag that the acceptance tests currently all fail with:

    Error: Could not load plugin
    
    
    Plugin reinitialization required. Please run "terraform init".
    
    Plugins are external binaries that Terraform uses to access and manipulate
    resources. The configuration provided requires plugins which can't be located,
    don't satisfy the version constraints, or are otherwise incompatible.
    
    Terraform automatically discovers provider requirements from your
    configuration, including providers used in child modules. To see the
    requirements and constraints, run "terraform providers".
    
    Failed to instantiate provider "registry.terraform.io/hashicorp/digitalocean"
    to obtain schema: unknown provider
    "registry.terraform.io/hashicorp/digitalocean"

Looks like resource.TestCase.Providers need to be replaced by resource.TestCase.ProviderFactories This doesn't seem well documented, but this should be a working example:

https://github.com/paultyng/terraform-provider-unifi/blob/aecb90da17c5269b7f2a859b7e1c0c204815ed23/internal/provider/provider_test.go#L13

---

One other thing we need to consider is that providers built with v2 of the SDK are incompatible with Terraform 0.11 cli. As a result, HashiCorp recommends the provider make a major release when doing this upgrade.

https://github.com/hashicorp/tf-sdk-migrator#tf-sdk-migrator-v2upgrade-migrate-from-sdkv1-to-sdkv2

As part of the ongoing deprecation of Terraform 0.11, version 2.0.0 of the Terraform Plugin SDK can only be used to build providers with support for Terraform 0.12 and higher.

It is recommended, but not required, that providers that have not already dropped support for 0.11 issue a major version bump when upgrading to version 2 of the SDK, to indicate this dropped support.

[tdyas]

Looks like resource.TestCase.Providers need to be replaced by resource.TestCase.ProviderFactories This doesn't seem well documented, but this should be a working example:

Pushed a change to add ProviderFactories to all of the tests.

I was able to run TestAccDigitalOceanRegions_Basic and fix an issue with it after also fixing several validation errors in the provider schemas that Terraform listed as errors at runtime during provider validation.

[andrewsomething]

So one thing I'm seeing is that we have a lot of tests that need to use a client directly to access the API and verify the existence of a resource. They use a pattern like:

client := testAccProvider.Meta().(*CombinedConfig).godoClient()

These are all panicking with:

panic: interface conversion: interface {} is nil, not *digitalocean.CombinedConfig

testAccProvider is not longer providing a configured client. In fact, it looks like it could be removed completely. We should be able to access the client with something lie:

provider, err := providerFactories["digitalocean"]()
if err != nil {
	return err
}
client := provider.Meta().(*CombinedConfig).godoClient()

---

Also, not entirely sure what happened, but there seems to be a lot of noise in the diff for the vendor directory. GitHub

[tdyas]

The AWS provider has been ported to the v2 SDK. We probably should use a similar pattern to how they consistently initialize the provider and the ProviderFactories to the same instance: https://github.com/terraform-providers/terraform-provider-aws/blob/8756a697db23a964e4af16b950af94e7c26885ab/aws/provider_test.go#L34-L59

[tdyas]

Rebased and squashed the commits. Found a straggler reference to the v1 SDK in the sort support in datalist package. Also fixed the provider setup in tests to set testAccProvider so that testAccProvider.Meta() works; test it by running make testacc TESTARGS='-run=^TestAccDataSourceDigitalOceanDomain_Basic'.

[andrewsomething]

This is fine in master, but causes a failure on all project related tests in this branch:

https://github.com/digitalocean/terraform-provider-digitalocean/blob/master/digitalocean/resource_digitalocean_project.go#L146

panic: Invalid address to set: []string{"id"}

goroutine 242 [running]:
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*ResourceData).Set(0xc0004d6d80, 0x13bfff9, 0x2, 0x1194180, 0xc0005226f0, 0xc0007223f0, 0xc000486640)
	/home/asb/development/gocode/pkg/mod/github.com/hashicorp/terraform-plugin-sdk/v2@v2.0.3/helper/schema/resource_data.go:188 +0x388
github.com/digitalocean/terraform-provider-digitalocean/digitalocean.resourceDigitalOceanProjectRead(0xc0004d6d80, 0x12289e0, 0xc00026ef60, 0x1, 0x1)
	/home/asb/development/gocode/src/github.com/digitalocean/terraform-provider-digitalocean/digitalocean/resource_digitalocean_project.go:147 +0x3f7
github.com/digitalocean/terraform-provider-digitalocean/digitalocean.resourceDigitalOceanProjectCreate(0xc0004d6d80, 0x12289e0, 0xc00026ef60, 0x0, 0xffffffffffffffff)
	/home/asb/development/gocode/src/github.com/digitalocean/terraform-provider-digitalocean/digitalocean/resource_digitalocean_project.go:129 +0x57d
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).create(0xc0002520b0, 0x16de7c0, 0xc00039e200, 0xc0004d6d80, 0x12289e0, 0xc00026ef60, 0x0, 0x0, 0x0)

[andrewsomething]

Another panic that passes on master:

panic: droplet_id: '' expected type 'string', got unconvertible type 'int'

goroutine 23475 [running]:
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*ResourceData).Set(0xc000842a80, 0x13c7831, 0xa, 0x11935c0, 0xc0003a3680, 0x0, 0x0)
	/home/asb/development/gocode/pkg/mod/github.com/hashicorp/terraform-plugin-sdk/v2@v2.0.3/helper/schema/resource_data.go:188 +0x388
github.com/digitalocean/terraform-provider-digitalocean/digitalocean.resourceDigitalOceanDropletSnapshotCreate(0xc000842a80, 0x12289e0, 0xc000c9f4d0, 0x0, 0xffffffffffffffff)
	/home/asb/development/gocode/src/github.com/digitalocean/terraform-provider-digitalocean/digitalocean/resource_digitalocean_droplet_snapshot.go:80 +0x54a

Looks like this is a real bug that should be fixed either way:

https://github.com/digitalocean/terraform-provider-digitalocean/blob/master/digitalocean/resource_digitalocean_droplet_snapshot.go#L80

[tdyas]

The panics are occurring in tests now now because the v2 SDK changes the .Set calls on ResourceData to panic now during tests instead of returning an error. https://www.terraform.io/docs/extend/guides/v2-upgrade-guide.html#louder-helper-schema-resourcedata-set-errors-in-testing

So clearly uncovering bugs as the change was made to do!

[andrewsomething]

@tdyas Definitely!

After working through those, I'm running into a number of failures to datasource tests that pass on master where the problem is not immediately clear to me. For example TestAccDataSourceDigitalOceanDomain_Basic consistently fails. Running it with debug logging, you can see the resource from the config is never even attempted to be created.

https://gist.github.com/andrewsomething/84fa93f00dbaebaf4ea1d5b974206693

[tdyas]

After working through those, I'm running into a number of failures to datasource tests that pass on master where the problem is not immediately clear to me. For example TestAccDataSourceDigitalOceanDomain_Basic consistently fails. Running it with debug logging, you can see the resource from the config is never even attempted to be created.

I've seen this issue when I was writing tests for some prior contributions. I had to first create the resource in a separate test step (without any checks) and then do a second test step with that resource still there and the added data source:

terraform-provider-digitalocean/digitalocean/datasource_digitalocean_projects_test.go

Lines 65 to 67 in c932c35

	{
	Config: resourcesConfig,
	},

[tdyas]

I fixed the tests for digitalocean_certificate and digitalocean_container_registry data sources. I ran the test for digitalocean_certificate and it passes. If the fix for digitalocean_container_registry works as well, I can fix the other failing tests in a similar manner.

[tdyas]

hashicorp/terraform-plugin-sdk#196
Looks like the solution is about to land upstream: hashicorp/terraform-plugin-sdk#614

Probably worth vendoring those set helpers and then switching to the SDK version once that PR actually lands and is released.

[tdyas]

@andrewsomething: I brought the PR branch up to your latest commit on the other branch. I also vendored the TypeSet helpers as internal/setutil (from https://github.com/terraform-providers/terraform-provider-aws/tree/master/aws/internal/tfawsresource). This should make it easy to switch to the SDK version once the applicable PR lands, but still fix the set issues in the meantime.

[andrewsomething]

👍 Thanks for all your work on this @tdyas!