Add ServiceNow extension resource #348
Conversation
drastawi
force-pushed
the
service-now-extension
branch
from
749f882 to
deab1b6
Compare
There was a problem hiding this comment.
Thanks for submitting this! I like the direction of this PR. I think it's probably cleaner for extensions like ServiceNow to have their own resource. However, there are several errors in the tests. I've pointed out a few that I see. If you could fix those I'd be happy to merge.
| resource.TestCheckResourceAttr( | ||
| "pagerduty_extension_servicenow.foo", "endpoint_url", url), | ||
| resource.TestCheckResourceAttr( | ||
| "pagerduty_extension_servicenow.foo", "config", "{\"notify_types\":{\"acknowledge\":false,\"assignments\":false,\"resolve\":false},\"restrict\":\"any\"}"), |
There was a problem hiding this comment.
There is no longer a config attribute in this resource. Please break out these checks to match the current schema of the resource.
| resource.TestCheckResourceAttr( | ||
| "pagerduty_extension_servicenow.foo", "endpoint_url", url_updated), | ||
| resource.TestCheckResourceAttr( | ||
| "pagerduty_extension_servicenow.foo", "config", "{\"notify_types\":{\"acknowledge\":true,\"assignments\":true,\"resolve\":true},\"restrict\":\"pd-users\"}"), |
There was a problem hiding this comment.
Same as above. Please break out these checks to match the current schema
| resource "pagerduty_extension_servicenow" "foo"{ | ||
| name = "%s" | ||
| endpoint_url = "%s" | ||
| extension_schema = data.pagerduty_extension_servicenow_schema.foo.id |
There was a problem hiding this comment.
I think this was supposed to be referencing data.pagerduty_extension_schema.foo.id
| snow_user = "meeps" | ||
| snow_password = "zorz" | ||
| sync_options = "manual_sync" | ||
| target = foo.servicenow.com/webhook_foo |
There was a problem hiding this comment.
I think the target value is supposed to be a string.
| resource "pagerduty_user" "example" { | ||
| name = "Howard James" | ||
| email = "howard.james@example.domain" | ||
| teams = [pagerduty_team.example.id] |
There was a problem hiding this comment.
The teams field is actually deprecated in the pagerduty_user resource. For these docs you can just remove the field.
| name = "My Web App" | ||
| auto_resolve_timeout = 14400 | ||
| acknowledgement_timeout = 600 | ||
| escalation_policy = pagerduty_escalation_policy.example.id |
There was a problem hiding this comment.
Since we're making other edits to this doc. This should probably match the definition above for pagerduty_escalation_policy.foo.id.
drastawi
force-pushed
the
service-now-extension
branch
from
deab1b6 to
151b186
Compare
|
Hey @stmcallister, I ran all the tests now against a live environment. Should all work. Also fixed the corresponding typos on the non-servicenow extension side. Let me know if it looks better now. |
Adding a dedicated resource for ServiceNow.
This resource will allow the end-user to avoid perma-diff and mark the password as a sensitive value while providing a simpler dedicated interface (e.g. endpoint URL can be very easily mistaken for the ServiceNow target).