[Update] Added Forbidden Response Check in PagerDuty Detector Verification

[nabeelalam]

Description:

Improved PagerDuty detector logic for credential validation.

Added handling for PagerDuty’s 403 Forbidden response for API tokens which are valid but do not have access to the resource:

{
  "error": {
    "message": "Access Denied",
    "code": 2010
  }
}

If the 403 response body contains the string "Access Denied" and the code "2010", the detector now treats this specific case as a successful credential validation (return true, nil).

Any other 403 response will return an unverified response with verification error since credential validity can’t be confirmed.

Checklist:

• Tests passing (make test-community)?
• Lint passing (make lint this requires golangci-lint)?

[martinlocklear]

Approving to unblock, but I think we should consider adding unit tests for this case.

[martinlocklear]

I wish there was unit test coverage for this. Also, does it make sense to actually parse the JSON, rather than to just do a string contains?

[nabeelalam]

Thanks for the review @martinlocklear, an additional unit test would be beneficial, yes.

Generally in detectors I've seen us rely on string contains, unless we need to extract something from the response. If our matching string is specific enough -- which I feel it is, in our case -- I lean towards string contains. IMO this would would also be slightly more adaptive to any potential change in the response shape (e.g. if keys get renamed but the message still contains these keywords)