Conversation
Signed-off-by: xenon898 <88700568+xenon898@users.noreply.github.com> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe Jira receiver documentation now describes OAuth2 system-to-system authentication as an alternative to personal access tokens and includes a corresponding ChangesJira OAuth2 documentation
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~3 minutes Merge Risk: ⚪ Minimal · up to The PR documents an already-supported Jira OAuth2 configuration without changing runtime behavior, so it is ready to merge. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
SoloJacobs
left a comment
There was a problem hiding this comment.
Hey,
I'm afraid this change is doing a bit more than it should:
- Jira doesn't actually support like OAuth2 without clicking in the browser. This means Alertmanager cannot do this either. What we can actually do is specify an idp via http config.
- The example defines a non-existing scope. We should probably remove that example altogether and point users to general http_config.
So, please make the change a bit more minimal. The main goal is just to check what may lead users to conclude one can't use OAuth with an idp. The OAuth solution by jira does not work for automations.
The
jira_configdocs only described PAT-based auth (basic_auth/authorization) forhttp_config, even though it's the same shared<http_config>type every notifier uses and already supports OAuth2 (verified againstOAuth2inprometheus/common@v0.70.1:client_id,client_secret_file,token_url,scopesare all real fields). That's what #5475 ran into.Adds a line noting OAuth2 is supported, and a worked example. The example uses independently-chosen placeholder values, not the one posted in the issue thread, since that suggestion was given as a plain comment rather than a signed-off contribution.
Fixes #5475
Pull Request Checklist
Which user-facing changes does this PR introduce?
🤖 Generated with Claude Code