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

Allow updating custom header values of webhook subscriptions #594

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pdecat
Copy link
Contributor

@pdecat pdecat commented Dec 9, 2022

This PR aims to allow updating custom header values of webhook subscriptions (AKA webhooks v3).

Currently, changes to the value field of these are completely ignored because:

To fix this situation, my plan is to:

  1. convert the custom_header field from schema.TypeList to schema.TypeSet, and let terraform compute a hash from the name and value for each header item,
  2. and stop storing the redacted value read from the API (there's no point in storing the redacted value anyway).
  3. We may add a check to store the value read if it differs from -- redacted -- (we never know, maybe the API will stop redacting all header values altogether at some point).

Alternatives considered:

  • Storing a hash of the headers name/value pairs in some field, and mark that field as ForceNew; true.
  • Request through PagerDuty support that the API behavior is modified to return a hash of the value instead of a static -- redacted -- value. Using a well-known and documented hash function would allow clients of the API to detect drift, and fix it.

That last option would be the best IMO, but I figured it will take time to implement on the PagerDuty API side,
and we need a solution in the short term to at least be able to update values.

@pdecat
Copy link
Contributor Author

pdecat commented Dec 9, 2022

Right now, I've only added a test case to demonstrate custom header values cannot be updated:

# make testacc TEST=./pagerduty/ TESTARGS='-run=TestAccPagerDutyWebhookSubscription'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./pagerduty/ -v -run=TestAccPagerDutyWebhookSubscription -timeout 120m
=== RUN   TestAccPagerDutyWebhookSubscription_import
--- PASS: TestAccPagerDutyWebhookSubscription_import (13.63s)
=== RUN   TestAccPagerDutyWebhookSubscription_Basic
    resource_pagerduty_webhook_subscription_test.go:55: Step 2/2 error: Expected a non-empty plan, but got an empty plan
--- FAIL: TestAccPagerDutyWebhookSubscription_Basic (16.46s)
FAIL
FAIL    github.com/terraform-providers/terraform-provider-pagerduty/pagerduty   30.102s
FAIL
make: *** [GNUmakefile:17: testacc] Error 1

Other commits to fix the situation are coming...

@pdecat pdecat changed the title Allow updating custom headers of webhook subscriptions Allow updating custom header values of webhook subscriptions Dec 9, 2022
@pdecat
Copy link
Contributor Author

pdecat commented Dec 9, 2022

Hi @stmcallister @imjaroiswebdev, while I'm looking at implementing this change, WDYT about the underlying issue?

@pdecat
Copy link
Contributor Author

pdecat commented Jan 9, 2023

Hi, gentle ping after the holiday season 🙏

@pdecat
Copy link
Contributor Author

pdecat commented Jun 8, 2023

Hi @stmcallister @imjaroiswebdev, gentle ping six months after 🙂

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.

None yet

1 participant