Upgrade Terraform Plugin SDK to v2 #375
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pdecat
commented
Aug 18, 2021
| "send_short_email": { | ||
| Type: schema.TypeBool, | ||
| Computed: true, | ||
| }, |
There was a problem hiding this comment.
These attributes were already documented but not implemented (and SDK v2 no longer accepts assignments to inexistent attributes)
|
Marked as ready for review as current state is not too bad when running all acceptance tests: The There's an instability in |
|
The Somehow, the acceptance tests do not catch the issue on master but the API really does not take the As you can see, some fields like Submitted a PR in the API client library to fix them: heimweh/go-pagerduty#58 Edit: spoke too soon, the API call payload is cleaner with the fix from heimweh/go-pagerduty#58 but the response still does not contain the expected values for |
|
Submitted #377 to address all the issues with |
pdecat
commented
Aug 20, 2021
Comment on lines
-45
to
+53
| Elem: &schema.Resource{ | ||
| Schema: map[string]*schema.Schema{ | ||
| "id": { | ||
| Type: schema.TypeString, | ||
| Required: true, | ||
| }, | ||
| "type": { | ||
| Type: schema.TypeString, | ||
| Required: true, | ||
| ValidateFunc: validateValueFunc([]string{ | ||
| "email_contact_method", | ||
| "phone_contact_method", | ||
| "push_notification_contact_method", | ||
| "sms_contact_method", | ||
| }), | ||
| }, | ||
| }, | ||
| // Using the `Elem` block to define specific keys for the map is currently not possible. | ||
| // The workaround described in SDK documentation is to confirm the required keys are set when expanding the Map object inside the resource code. | ||
| // See https://www.terraform.io/docs/extend/schemas/schema-types.html#typemap | ||
| Elem: &schema.Schema{ | ||
| Type: schema.TypeString, | ||
| }, | ||
| ValidateDiagFunc: validation.MapKeyMatch(regexp.MustCompile("(id|type)"), "`contact_method` must only have `id` and `types` attributes"), |
There was a problem hiding this comment.
An alternative that would have avoided to deport the validation logic is to convert the contact_method attribute to a TypeList or TypeSet with both MinItems and MaxItems set to 1, but then this would have required configuration changes for existing users to replace contact_method = {} assignments by contact_method {} blocks (without the equal sign).
|
Rebased on master, @stmcallister PTAL. |
…ashicorp/terraform-json#28 and resolve `unsupported state format version: expected "0.1", got "0.2"` issues when running acceptance tests: Before: ``` make testacc TEST=./pagerduty TESTARGS='-run=TestAccPagerDutyUserWithTeams_Basic -count=1' ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./pagerduty -v -run=TestAccPagerDutyUserWithTeams_Basic -count=1 -timeout 120m === RUN TestAccPagerDutyUserWithTeams_Basic 2021/08/18 12:29:16 [WARN] Got error running Terraform: unsupported state format version: expected "0.1", got "0.2" resource_pagerduty_user_test.go:115: Step 1/3 error: unsupported state format version: expected "0.1", got "0.2" 2021/08/18 12:29:16 [WARN] Got error running Terraform: unsupported state format version: expected "0.1", got "0.2" testing_new.go:56: Error retrieving state, there may be dangling resources: unsupported state format version: expected "0.1", got "0.2" --- FAIL: TestAccPagerDutyUserWithTeams_Basic (0.58s) FAIL FAIL github.com/terraform-providers/terraform-provider-pagerduty/pagerduty 0.588s FAIL make: *** [GNUmakefile:17: testacc] Error 1 ``` After: ``` make testacc TEST=./pagerduty TESTARGS='-run=TestAccPagerDutyUserWithTeams_Basic -count=1' ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./pagerduty -v -run=TestAccPagerDutyUserWithTeams_Basic -count=1 -timeout 120m === RUN TestAccPagerDutyUserWithTeams_Basic --- PASS: TestAccPagerDutyUserWithTeams_Basic (21.24s) PASS ok github.com/terraform-providers/terraform-provider-pagerduty/pagerduty 21.255s ``` Commands: ``` go get -u github.com/hashicorp/terraform-plugin-sdk/v2 go mod tidy go mod vendor ```
…vertible type 'string'`
…` and other missing attributes
…summary` attribute
…tible type 'pagerduty.ExtensionSchemaReference'`
…`summary` attribute
…tible type 'pagerduty.ExtensionSchemaReference'`
```
=== RUN TestAccPagerDutyUserNotificationRuleContactMethod_Basic
resource_pagerduty_user_notification_rule_test.go:20: Step 1/3 error: Error running pre-apply refresh: exit status 1
Error: Insufficient contact_method blocks
on terraform_plugin_test.tf line 2, in resource "pagerduty_user_notification_rule" "foo":
2: resource "pagerduty_user_notification_rule" "foo" {
At least 1 "contact_method" blocks are required.
Error: Unsupported argument
on terraform_plugin_test.tf line 7, in resource "pagerduty_user_notification_rule" "foo":
7: contact_method = {
An argument named "contact_method" is not expected here. Did you mean to
define a block of type "contact_method"?
--- FAIL: TestAccPagerDutyUserNotificationRuleContactMethod_Basic (0.68s)
```
```
=== RUN TestAccPagerDutyUserNotificationRuleContactMethod_Basic
panic: Invalid address to set: []string{"type"}
goroutine 408 [running]:
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*ResourceData).Set(0xc000484a00, {0x1050f61, 0x4}, {0xec95a0, 0xc0004fc230})
/home/patrick/go/src/github.com/PagerDuty/terraform-provider-pagerduty/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema/resource_data.go:230 +0x2a5
github.com/terraform-providers/terraform-provider-pagerduty/pagerduty.resourcePagerDutyUserNotificationRuleRead.func1()
/home/patrick/go/src/github.com/PagerDuty/terraform-provider-pagerduty/pagerduty/resource_pagerduty_user_notification_rule.go:111 +0x1a5
github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource.RetryContext.func1()
/home/patrick/go/src/github.com/PagerDuty/terraform-provider-pagerduty/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource/wait.go:27 +0x56
github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource.(*StateChangeConf).WaitForStateContext.func1()
/home/patrick/go/src/github.com/PagerDuty/terraform-provider-pagerduty/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource/state.go:110 +0x21f
created by github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource.(*StateChangeConf).WaitForStateContext
/home/patrick/go/src/github.com/PagerDuty/terraform-provider-pagerduty/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource/state.go:83 +0x1dd
FAIL github.com/terraform-providers/terraform-provider-pagerduty/pagerduty 7.732s
```
…t_grouping_parameters is not defined as omitempty only works if the value is nil, not empty
…convertible type 'int'`:
```
panic: alert_grouping_timeout: '' expected type 'string', got unconvertible type 'int'
goroutine 288 [running]:
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*ResourceData).Set(0xc000188900, {0x1065e32, 0x16}, {0xec9ec0, 0xc0006aa8e8})
/home/patrick/go/src/github.com/PagerDuty/terraform-provider-pagerduty/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema/resource_data.go:230 +0x2a5
github.com/terraform-providers/terraform-provider-pagerduty/pagerduty.resourcePagerDutyServiceRead.func1()
/home/patrick/go/src/github.com/PagerDuty/terraform-provider-pagerduty/pagerduty/resource_pagerduty_service.go:396 +0x508
github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource.RetryContext.func1()
/home/patrick/go/src/github.com/PagerDuty/terraform-provider-pagerduty/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource/wait.go:27 +0x56
github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource.(*StateChangeConf).WaitForStateContext.func1()
/home/patrick/go/src/github.com/PagerDuty/terraform-provider-pagerduty/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource/state.go:110 +0x21f
created by github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource.(*StateChangeConf).WaitForStateContext
/home/patrick/go/src/github.com/PagerDuty/terraform-provider-pagerduty/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource/state.go:83 +0x1dd
FAIL github.com/terraform-providers/terraform-provider-pagerduty/pagerduty 5.947s
FAIL
make: *** [GNUmakefile:17: testacc] Error 1
```
…when not expected`:
```
=== RUN TestAccPagerDutyService_Basic
resource_pagerduty_service_test.go:57: Step 1/3 error: Check failed: Check 8/12 error: pagerduty_service.foo: Attribute 'alert_grouping_timeout' found when not expected
--- FAIL: TestAccPagerDutyService_Basic (11.33s)
```
…ce {}) error) as type schema.CustomizeDiffFunc in field value`
stmcallister
approved these changes
Oct 8, 2021
|
Thanks for your time and all the positive comments @stmcallister, very much appreciated! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR upgrades the provider code from Terraform Plugin SDK v1.7.0 to v2.7.0,
following recommendations from the Terraform Plugin SDK v2 Upgrade Guide.
By default,
tf-sdk-migrator v2upgradeupgrades the SDK to v2.4.3 but this version causedunsupported state format version: expected "0.1", got "0.2"issues when running acceptance tests, which SDK v2.7.0 and terraform-json v0.12.0 resolve as they include hashicorp/terraform-json#28.With SDK v2.4.3 and terraform-json v0.8.0:
With SDK v2.7.0 and terraform-json v0.12.0:
Submitted as draft as not all acceptance tests pass yet.