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

Upgrade Terraform Plugin SDK to v2 #375

Merged
merged 20 commits into from
Oct 8, 2021
Merged

Conversation

pdecat
Copy link
Contributor

@pdecat pdecat commented Aug 18, 2021

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 v2upgrade upgrades the SDK to v2.4.3 but this version caused unsupported 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:

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

With SDK v2.7.0 and terraform-json v0.12.0:

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

Submitted as draft as not all acceptance tests pass yet.

"send_short_email": {
Type: schema.TypeBool,
Computed: true,
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These attributes were already documented but not implemented (and SDK v2 no longer accepts assignments to inexistent attributes)

@pdecat pdecat marked this pull request as ready for review August 20, 2021 06:58
@pdecat
Copy link
Contributor Author

pdecat commented Aug 20, 2021

Marked as ready for review as current state is not too bad when running all acceptance tests:

# make testacc TEST=./pagerduty TESTARGS='-count=1'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./pagerduty -v -count=1 -timeout 120m
=== RUN   TestConfigEmptyToken
--- PASS: TestConfigEmptyToken (0.00s)
=== RUN   TestConfigSkipCredsValidation
2021/08/20 08:40:16 ===== PagerDuty Cache Skipping Init =====
2021/08/20 08:40:16 [INFO] PagerDuty client configured
--- PASS: TestConfigSkipCredsValidation (0.00s)
=== RUN   TestAccDataSourcePagerDutyBusinessService_Basic
--- PASS: TestAccDataSourcePagerDutyBusinessService_Basic (7.08s)
=== RUN   TestAccDataSourcePagerDutyEscalationPolicy_Basic
--- PASS: TestAccDataSourcePagerDutyEscalationPolicy_Basic (11.61s)
=== RUN   TestAccDataSourcePagerDutyExtensionSchema_Basic
--- PASS: TestAccDataSourcePagerDutyExtensionSchema_Basic (4.69s)
=== RUN   TestAccDataSourcePagerDutyPriority_Basic
    data_source_pagerduty_priority_test.go:11: Step 1/1 error: Error running pre-apply refresh: exit status 1

        Error: GET API call to https://api.pagerduty.com/priorities failed: 404 Not Found

          with data.pagerduty_priority.p1,
          on terraform_plugin_test.tf line 2, in data "pagerduty_priority" "p1":
           2: data "pagerduty_priority" "p1" {

--- FAIL: TestAccDataSourcePagerDutyPriority_Basic (1.18s)
=== RUN   TestAccDataSourcePagerDutyPriority_P2
    data_source_pagerduty_priority_test.go:27: Step 1/1 error: Error running pre-apply refresh: exit status 1

        Error: GET API call to https://api.pagerduty.com/priorities failed: 404 Not Found

          with data.pagerduty_priority.p2,
          on terraform_plugin_test.tf line 2, in data "pagerduty_priority" "p2":
           2: data "pagerduty_priority" "p2" {

--- FAIL: TestAccDataSourcePagerDutyPriority_P2 (1.25s)
=== RUN   TestAccDataSourcePagerDutyRuleset_Basic
--- PASS: TestAccDataSourcePagerDutyRuleset_Basic (6.00s)
=== RUN   TestAccDataSourcePagerDutySchedule_Basic
--- PASS: TestAccDataSourcePagerDutySchedule_Basic (11.01s)
=== RUN   TestAccDataSourcePagerDutyIntegration_Basic
--- PASS: TestAccDataSourcePagerDutyIntegration_Basic (21.39s)
=== RUN   TestAccDataSourcePagerDutyService_Basic
--- PASS: TestAccDataSourcePagerDutyService_Basic (13.99s)
=== RUN   TestAccDataSourcePagerDutyTeam_Basic
--- PASS: TestAccDataSourcePagerDutyTeam_Basic (8.27s)
=== RUN   TestAccDataSourcePagerDutyUserContactMethod_Basic
--- PASS: TestAccDataSourcePagerDutyUserContactMethod_Basic (10.39s)
=== RUN   TestAccDataSourcePagerDutyUser_Basic
--- PASS: TestAccDataSourcePagerDutyUser_Basic (9.31s)
=== RUN   TestAccDataSourcePagerDutyVendor_Basic
--- PASS: TestAccDataSourcePagerDutyVendor_Basic (4.62s)
=== RUN   TestAccDataSourcePagerDutyVendor_ExactMatch
--- PASS: TestAccDataSourcePagerDutyVendor_ExactMatch (4.75s)
=== RUN   TestAccDataSourcePagerDutyVendor_SpecialChars
--- PASS: TestAccDataSourcePagerDutyVendor_SpecialChars (4.69s)
=== RUN   TestAccPagerDutyBusinessService_import
--- PASS: TestAccPagerDutyBusinessService_import (6.36s)
=== RUN   TestAccPagerDutyEscalationPolicy_import
--- PASS: TestAccPagerDutyEscalationPolicy_import (11.98s)
=== RUN   TestAccPagerDutyEventRule_import
--- PASS: TestAccPagerDutyEventRule_import (6.69s)
=== RUN   TestAccPagerDutyExtensionServiceNow_import
--- PASS: TestAccPagerDutyExtensionServiceNow_import (15.92s)
=== RUN   TestAccPagerDutyExtension_import
--- PASS: TestAccPagerDutyExtension_import (16.78s)
=== RUN   TestAccPagerDutyMaintenanceWindow_import
--- PASS: TestAccPagerDutyMaintenanceWindow_import (15.21s)
=== RUN   TestAccPagerDutyResponsePlay_import
--- PASS: TestAccPagerDutyResponsePlay_import (13.13s)
=== RUN   TestAccPagerDutyRulesetRule_import
--- PASS: TestAccPagerDutyRulesetRule_import (10.78s)
=== RUN   TestAccPagerDutyRuleset_import
--- PASS: TestAccPagerDutyRuleset_import (8.83s)
=== RUN   TestAccPagerDutyRulesetWithNoTeam_import
--- PASS: TestAccPagerDutyRulesetWithNoTeam_import (6.40s)
=== RUN   TestAccPagerDutySchedule_import
--- PASS: TestAccPagerDutySchedule_import (11.33s)
=== RUN   TestAccPagerDutyServiceDependency_import
--- PASS: TestAccPagerDutyServiceDependency_import (20.16s)
=== RUN   TestAccPagerDutyServiceEventRule_import
--- PASS: TestAccPagerDutyServiceEventRule_import (17.05s)
=== RUN   TestAccPagerDutyServiceIntegration_import
--- PASS: TestAccPagerDutyServiceIntegration_import (17.06s)
=== RUN   TestAccPagerDutyService_import
--- PASS: TestAccPagerDutyService_import (14.54s)
=== RUN   TestAccPagerDutyServiceWithIncidentUrgency_import
--- PASS: TestAccPagerDutyServiceWithIncidentUrgency_import (13.80s)
=== RUN   TestAccPagerDutyTeamMembership_import
--- PASS: TestAccPagerDutyTeamMembership_import (10.87s)
=== RUN   TestAccPagerDutyTeamMembership_importWithRole
--- PASS: TestAccPagerDutyTeamMembership_importWithRole (11.46s)
=== RUN   TestAccPagerDutyTeam_import
--- PASS: TestAccPagerDutyTeam_import (7.81s)
=== RUN   TestAccPagerDutyUserContactMethod_import
--- PASS: TestAccPagerDutyUserContactMethod_import (10.65s)
=== RUN   TestAccPagerDutyUserNotificationRule_import
--- PASS: TestAccPagerDutyUserNotificationRule_import (12.92s)
=== RUN   TestAccPagerDutyUser_import
--- PASS: TestAccPagerDutyUser_import (9.40s)
=== RUN   TestProvider
--- PASS: TestProvider (0.01s)
=== RUN   TestProviderImpl
--- PASS: TestProviderImpl (0.00s)
=== RUN   TestAccPagerDutyAddon_Basic
--- PASS: TestAccPagerDutyAddon_Basic (9.77s)
=== RUN   TestAccPagerDutyBusinessService_Basic
--- PASS: TestAccPagerDutyBusinessService_Basic (9.54s)
=== RUN   TestAccPagerDutyBusinessService_WithTeam
--- PASS: TestAccPagerDutyBusinessService_WithTeam (8.47s)
=== RUN   TestAccPagerDutyEscalationPolicy_Basic
--- PASS: TestAccPagerDutyEscalationPolicy_Basic (16.44s)
=== RUN   TestAccPagerDutyEscalationPolicyWithTeams_Basic
--- PASS: TestAccPagerDutyEscalationPolicyWithTeams_Basic (17.04s)
=== RUN   TestAccPagerDutyEventRule_Basic
--- PASS: TestAccPagerDutyEventRule_Basic (11.04s)
=== RUN   TestAccPagerDutyExtensionServiceNow_Basic
--- PASS: TestAccPagerDutyExtensionServiceNow_Basic (22.08s)
=== RUN   TestAccPagerDutyExtension_Basic
--- PASS: TestAccPagerDutyExtension_Basic (23.06s)
=== RUN   TestAccPagerDutyMaintenanceWindow_Basic
--- PASS: TestAccPagerDutyMaintenanceWindow_Basic (21.98s)
=== RUN   TestAccPagerDutyResponsePlay_Basic
--- PASS: TestAccPagerDutyResponsePlay_Basic (17.67s)
=== RUN   TestAccPagerDutyRulesetRule_Basic
--- PASS: TestAccPagerDutyRulesetRule_Basic (14.90s)
=== RUN   TestAccPagerDutyRulesetRule_MultipleRules
--- PASS: TestAccPagerDutyRulesetRule_MultipleRules (12.52s)
=== RUN   TestAccPagerDutyRuleset_Basic
--- PASS: TestAccPagerDutyRuleset_Basic (17.79s)
=== RUN   TestAccPagerDutySchedule_Basic
--- PASS: TestAccPagerDutySchedule_Basic (16.08s)
=== RUN   TestAccPagerDutyScheduleWithTeams_Basic
--- PASS: TestAccPagerDutyScheduleWithTeams_Basic (17.32s)
=== RUN   TestAccPagerDutyScheduleOverflow_Basic
--- PASS: TestAccPagerDutyScheduleOverflow_Basic (15.50s)
=== RUN   TestAccPagerDutySchedule_BasicWeek
--- PASS: TestAccPagerDutySchedule_BasicWeek (15.92s)
=== RUN   TestAccPagerDutySchedule_Multi
--- PASS: TestAccPagerDutySchedule_Multi (16.77s)
=== RUN   TestAccPagerDutyBusinessServiceDependency_Basic
--- PASS: TestAccPagerDutyBusinessServiceDependency_Basic (33.09s)
=== RUN   TestAccPagerDutyTechnicalServiceDependency_Basic
--- PASS: TestAccPagerDutyTechnicalServiceDependency_Basic (18.71s)
=== RUN   TestAccPagerDutyServiceEventRule_Basic
--- PASS: TestAccPagerDutyServiceEventRule_Basic (21.98s)
=== RUN   TestAccPagerDutyServiceEventRule_MultipleRules
--- PASS: TestAccPagerDutyServiceEventRule_MultipleRules (17.43s)
=== RUN   TestAccPagerDutyServiceIntegration_Basic
--- PASS: TestAccPagerDutyServiceIntegration_Basic (23.29s)
=== RUN   TestAccPagerDutyServiceIntegrationGeneric_Basic
--- PASS: TestAccPagerDutyServiceIntegrationGeneric_Basic (21.88s)
=== 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.09s)
=== RUN   TestAccPagerDutyService_AlertGrouping
    resource_pagerduty_service_test.go:136: Step 1/2 error: Check failed: Check 8/11 error: pagerduty_service.foo: Attribute 'alert_grouping_timeout' expected "1800", got "null"
--- FAIL: TestAccPagerDutyService_AlertGrouping (11.49s)
=== RUN   TestAccPagerDutyService_AlertContentGrouping
--- PASS: TestAccPagerDutyService_AlertContentGrouping (13.47s)
=== RUN   TestAccPagerDutyService_BasicWithIncidentUrgencyRules
--- PASS: TestAccPagerDutyService_BasicWithIncidentUrgencyRules (24.95s)
=== RUN   TestAccPagerDutyService_FromBasicToCustomIncidentUrgencyRules
--- PASS: TestAccPagerDutyService_FromBasicToCustomIncidentUrgencyRules (18.37s)
=== RUN   TestAccPagerDutyService_SupportHoursChange
--- PASS: TestAccPagerDutyService_SupportHoursChange (19.59s)
=== RUN   TestAccPagerDutyTeamMembership_Basic
--- PASS: TestAccPagerDutyTeamMembership_Basic (10.70s)
=== RUN   TestAccPagerDutyTeamMembership_WithRole
--- PASS: TestAccPagerDutyTeamMembership_WithRole (10.94s)
=== RUN   TestAccPagerDutyTeam_Basic
--- PASS: TestAccPagerDutyTeam_Basic (12.04s)
=== RUN   TestAccPagerDutyTeam_Parent
--- PASS: TestAccPagerDutyTeam_Parent (11.22s)
=== RUN   TestAccPagerDutyUserContactMethodEmail_Basic
--- PASS: TestAccPagerDutyUserContactMethodEmail_Basic (15.20s)
=== RUN   TestAccPagerDutyUserContactMethodPhone_Basic
--- PASS: TestAccPagerDutyUserContactMethodPhone_Basic (16.04s)
=== RUN   TestAccPagerDutyUserContactMethodSMS_Basic
--- PASS: TestAccPagerDutyUserContactMethodSMS_Basic (16.81s)
=== RUN   TestAccPagerDutyUserNotificationRuleContactMethod_Basic
--- PASS: TestAccPagerDutyUserNotificationRuleContactMethod_Basic (23.37s)
=== RUN   TestAccPagerDutyUserNotificationRuleContactMethod_Invalid
--- PASS: TestAccPagerDutyUserNotificationRuleContactMethod_Invalid (7.01s)
=== RUN   TestAccPagerDutyUserNotificationRuleContactMethod_Missing_id
--- PASS: TestAccPagerDutyUserNotificationRuleContactMethod_Missing_id (6.30s)
=== RUN   TestAccPagerDutyUserNotificationRuleContactMethod_Missing_type
--- PASS: TestAccPagerDutyUserNotificationRuleContactMethod_Missing_type (7.19s)
=== RUN   TestAccPagerDutyUserNotificationRuleContactMethod_Unknown_key
--- PASS: TestAccPagerDutyUserNotificationRuleContactMethod_Unknown_key (7.68s)
=== RUN   TestAccPagerDutyUser_Basic
--- PASS: TestAccPagerDutyUser_Basic (13.38s)
=== RUN   TestAccPagerDutyUserWithTeams_Basic
--- PASS: TestAccPagerDutyUserWithTeams_Basic (20.77s)
FAIL
FAIL    github.com/terraform-providers/terraform-provider-pagerduty/pagerduty   1087.276s
FAIL
make: *** [GNUmakefile:17: testacc] Error 1

The TestAccDataSourcePagerDutyPriority_Basic and TestAccDataSourcePagerDutyPriority_P2 failures are caused by development accounts, see #219 (comment)

There's an instability in TestAccPagerDutyServiceDependency_import causing crashes that's addressed in #376 as it also happens on current master (it required several retries to get a non crashing full acceptance tests run here).

@pdecat
Copy link
Contributor Author

pdecat commented Aug 20, 2021

The alert_grouping_timeout failures are caused by heimweh/go-pagerduty#57:

=== 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.09s)
=== RUN   TestAccPagerDutyService_AlertGrouping
    resource_pagerduty_service_test.go:136: Step 1/2 error: Check failed: Check 8/11 error: pagerduty_service.foo: Attribute 'alert_grouping_timeout' expected "1800", got "null"
--- FAIL: TestAccPagerDutyService_AlertGrouping (11.49s)

Somehow, the acceptance tests do not catch the issue on master but the API really does not take the alert_grouping setting into account:

2021/08/20 09:17:11 [INFO] Creating PagerDuty service tf-d60pe
2021/08/20 09:17:11 [DEBUG] PagerDuty - Preparing POST request to /services with body: {"acknowledgement_timeout":null,"alert_grouping":null,"auto_resolve_timeout":null,"service":{"acknowledgement_timeout":1800,"alert_creation":"create_alerts_and_incidents","alert_grouping":"time","alert_grouping_timeout":1800,"alert_grouping_parameters":{},"auto_resolve_timeout":1800,"description":"foo","escalation_policy":{"id":"PX530JT","type":"escalation_policy_reference"},"name":"tf-d60pe"}}
2021/08/20 09:17:11 [DEBUG] PagerDuty API Request Details:
---[ REQUEST ]---------------------------------------
POST /services HTTP/1.1
Host: api.pagerduty.com
User-Agent: heimweh/go-pagerduty(terraform)
Content-Length: 398
Accept: application/vnd.pagerduty+json;version=2
Authorization: Token token=******
Content-Type: application/json
Accept-Encoding: gzip

{
 "acknowledgement_timeout": null,
 "alert_grouping": null,
 "auto_resolve_timeout": null,
 "service": {
  "acknowledgement_timeout": 1800,
  "alert_creation": "create_alerts_and_incidents",
  "alert_grouping": "time",
  "alert_grouping_timeout": 1800,
  "alert_grouping_parameters": {},
  "auto_resolve_timeout": 1800,
  "description": "foo",
  "escalation_policy": {
   "id": "PX530JT",
   "type": "escalation_policy_reference"
  },
  "name": "tf-d60pe"
 }
}

-----------------------------------------------------
2021/08/20 09:17:11 [DEBUG] PagerDuty API Response Details:
---[ RESPONSE ]--------------------------------------
HTTP/2.0 201 Created
Content-Length: 976
Access-Control-Allow-Headers: Authorization, Content-Type, AuthorizationOauth, X-EARLY-ACCESS
Access-Control-Allow-Methods: GET, POST, PUT, DELETE, OPTIONS
Access-Control-Allow-Origin: *
Access-Control-Expose-Headers: 
Access-Control-Max-Age: 1728000
Cache-Control: max-age=0, private, must-revalidate
Content-Type: application/json
Date: Fri, 20 Aug 2021 07:17:11 GMT
Etag: W/"517f34e20ed8089259a734bb0137b371"
Feature-Policy: accelerometer 'none'; camera 'none'; geolocation 'none'; gyroscope 'none'; magnetometer 'none'; microphone 'none'; payment 'none'; usb 'none'
Referrer-Policy: strict-origin-when-cross-origin
Server: nginx
Strict-Transport-Security: max-age=31536000; includeSubDomains
X-Content-Type-Options: nosniff
X-Request-Id: 513fe44f47c4cc052001d152acb18a20
X-Xss-Protection: 1; mode=block

{
 "service": {
  "id": "P71DGB9",
  "name": "tf-d60pe",
  "description": "foo",
  "created_at": "2021-08-20T09:17:11+02:00",
  "updated_at": "2021-08-20T09:17:11+02:00",
  "status": "active",
  "teams": [],
  "alert_creation": "create_alerts_and_incidents",
  "addons": [],
  "scheduled_actions": [],
  "support_hours": null,
  "last_incident_timestamp": null,
  "escalation_policy": {
   "id": "PX530JT",
   "type": "escalation_policy_reference",
   "summary": "tf-lrua3",
   "self": "https://api.pagerduty.com/escalation_policies/PX530JT",
   "html_url": "https://dev-claranet.pagerduty.com/escalation_policies/PX530JT"
  },
  "incident_urgency_rule": {
   "type": "constant",
   "urgency": "high"
  },
  "acknowledgement_timeout": 1800,
  "auto_resolve_timeout": 1800,
  "alert_grouping": null,
  "alert_grouping_timeout": null,
  "alert_grouping_parameters": {
   "type": null,
   "config": null
  },
  "integrations": [],
  "response_play": null,
  "type": "service",
  "summary": "tf-d60pe",
  "self": "https://api.pagerduty.com/services/P71DGB9",
  "html_url": "https://dev-claranet.pagerduty.com/service-directory/P71DGB9"
 }
}
-----------------------------------------------------

As you can see, some fields like alert_grouping are passed twice, once at the root defined as null and once in the nested service attribute as alert_grouping, and the response only returns the one at the root.

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 alert_grouping:

2021/08/20 10:25:34 [INFO] Creating PagerDuty service tf-apgg1
2021/08/20 10:25:34 [DEBUG] PagerDuty - Preparing POST request to /services with body: {"service":{"acknowledgement_timeout":1800,"alert_creation":"create_alerts_and_incidents","alert_grouping":"time","alert_grouping_timeout":1800,"alert_grouping_parameters":{},"auto_resolve_timeout":1800,"description":"foo","escalation_policy":{"id":"PV1994B","type":"escalation_policy_reference"},"name":"tf-apgg1"}}
2021/08/20 10:25:34 [DEBUG] PagerDuty API Request Details:
---[ REQUEST ]---------------------------------------
POST /services HTTP/1.1
Host: api.pagerduty.com
User-Agent: heimweh/go-pagerduty(terraform)
Content-Length: 317
Accept: application/vnd.pagerduty+json;version=2
Authorization: Token token=******
Content-Type: application/json
Accept-Encoding: gzip

{
 "service": {
  "acknowledgement_timeout": 1800,
  "alert_creation": "create_alerts_and_incidents",
  "alert_grouping": "time",
  "alert_grouping_timeout": 1800,
  "alert_grouping_parameters": {},
  "auto_resolve_timeout": 1800,
  "description": "foo",
  "escalation_policy": {
   "id": "PV1994B",
   "type": "escalation_policy_reference"
  },
  "name": "tf-apgg1"
 }
}

-----------------------------------------------------
2021/08/20 10:25:35 [DEBUG] PagerDuty API Response Details:
---[ RESPONSE ]--------------------------------------
HTTP/2.0 201 Created
Content-Length: 976
Access-Control-Allow-Headers: Authorization, Content-Type, AuthorizationOauth, X-EARLY-ACCESS
Access-Control-Allow-Methods: GET, POST, PUT, DELETE, OPTIONS
Access-Control-Allow-Origin: *
Access-Control-Expose-Headers: 
Access-Control-Max-Age: 1728000
Cache-Control: max-age=0, private, must-revalidate
Content-Type: application/json
Date: Fri, 20 Aug 2021 08:25:35 GMT
Etag: W/"8f830e92001c921b4211c6b10dc447b0"
Feature-Policy: accelerometer 'none'; camera 'none'; geolocation 'none'; gyroscope 'none'; magnetometer 'none'; microphone 'none'; payment 'none'; usb 'none'
Referrer-Policy: strict-origin-when-cross-origin
Server: nginx
Strict-Transport-Security: max-age=31536000; includeSubDomains
X-Content-Type-Options: nosniff
X-Request-Id: c49acd3ca2f2112885d34ec0f972d6d4
X-Xss-Protection: 1; mode=block

{
 "service": {
  "id": "P117OMI",
  "name": "tf-apgg1",
  "description": "foo",
  "created_at": "2021-08-20T10:25:35+02:00",
  "updated_at": "2021-08-20T10:25:35+02:00",
  "status": "active",
  "teams": [],
  "alert_creation": "create_alerts_and_incidents",
  "addons": [],
  "scheduled_actions": [],
  "support_hours": null,
  "last_incident_timestamp": null,
  "escalation_policy": {
   "id": "PV1994B",
   "type": "escalation_policy_reference",
   "summary": "tf-8g1f0",
   "self": "https://api.pagerduty.com/escalation_policies/PV1994B",
   "html_url": "https://dev-claranet.pagerduty.com/escalation_policies/PV1994B"
  },
  "incident_urgency_rule": {
   "type": "constant",
   "urgency": "high"
  },
  "acknowledgement_timeout": 1800,
  "auto_resolve_timeout": 1800,
  "alert_grouping": null,
  "alert_grouping_timeout": null,
  "alert_grouping_parameters": {
   "type": null,
   "config": null
  },
  "integrations": [],
  "response_play": null,
  "type": "service",
  "summary": "tf-apgg1",
  "self": "https://api.pagerduty.com/services/P117OMI",
  "html_url": "https://dev-claranet.pagerduty.com/service-directory/P117OMI"
 }
}
-----------------------------------------------------
2021/08/20 10:25:35 [INFO] Reading PagerDuty service P117OMI

@pdecat
Copy link
Contributor Author

pdecat commented Aug 20, 2021

Submitted #377 to address all the issues with alert_grouping and alert_grouping_timeout that were already present in master but not detected.

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"),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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).

@pdecat
Copy link
Contributor Author

pdecat commented Sep 10, 2021

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
```
…tible type 'pagerduty.ExtensionSchemaReference'`
…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`
Copy link
Contributor

@stmcallister stmcallister left a comment

Choose a reason for hiding this comment

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

This is incredible! Thank you for updating the SDK and resolving all of the related issues. 🎉

@stmcallister stmcallister merged commit f2f4935 into PagerDuty:master Oct 8, 2021
@pdecat pdecat deleted the tf_sdk_v2 branch October 8, 2021 06:39
@pdecat
Copy link
Contributor Author

pdecat commented 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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants