fix(jira): make labels field optional when not configured#5263
fix(jira): make labels field optional when not configured#5263Cloud-Architect-Emma wants to merge 2 commits into
Conversation
Fixes prometheus#5230 Previously, the issue fields struct always initialized Labels as a non-nil empty slice (make([]string, 0, ...)), causing the Jira API to receive a 'labels' field even when no labels were configured. Jira screens that do not include the Labels field reject such requests with 'Field labels cannot be set'. Initialize Labels as nil so the field is omitted from the JSON payload when not configured by the user. Co-authored-by: Cloud-Architect-Emma <Cloud-Architect-Emma@users.noreply.github.com> Signed-off-by: Cloud-Architect-Emma <emmanuela_prince@yahoo.com>
📝 WalkthroughWalkthroughJIRA issue Labels field is now optional: it initializes to nil and is only populated (templated, appended with ALERT{groupID}, and sorted) when configured labels are provided; minor comment and brace-position edits elsewhere do not change behavior. ChangesJIRA Labels Optional
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@notify/jira/jira.go`:
- Around line 195-205: The correlation label ALERT{groupID} is only appended
when n.conf.Labels != nil, which prevents searchExistingIssue from finding
issues when users omit labels; to fix, ensure requestBody.Fields.Labels is
initialized (make an empty slice if nil) and append the ALERT{groupID} label
outside/after the n.conf.Labels conditional so the correlation label is always
present; update code around n.conf.Labels, requestBody.Fields.Labels and the
ALERT{groupID} append site and keep searchExistingIssue's marker unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: eddccc55-1da2-49de-af25-fca68bacd343
📒 Files selected for processing (1)
notify/jira/jira.go
The ALERT{groupID} correlation label is always added so dedup/reopen/
resolve flows continue to work. User-configured labels are only added
when explicitly set, so Jira screens without a Labels field are not
affected when no labels are configured.
Co-authored-by: Cloud-Architect-Emma <Cloud-Architect-Emma@users.noreply.github.com>
Signed-off-by: Cloud-Architect-Emma <emmanuela_prince@yahoo.com>
|
Hi maintainers, could someone please approve the workflow run so the build and test checks can execute? The fix ensures the ALERT{groupID} correlation label is always set (preserving dedup/reopen/resolve flows) while only sending user-configured labels when explicitly set, so Jira screens without a Labels field are no longer affected. Thanks! |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
notify/jira/jira.go (1)
195-215:⚠️ Potential issue | 🟠 Major | ⚡ Quick winComment at lines 197-198 is inconsistent with the actual behavior.
The comment claims "Jira screens that do not include the Labels field are not affected when no labels are configured," but the else block at lines 210-214 always sets
requestBody.Fields.Labels = []string{alertLabel}. SinceLabelsis non-nil,MarshalJSONintypes.gowill include"labels": ["ALERT{...}"]in the JSON payload, causing Jira projects/screens without the Labels field to still fail with "Field 'labels' cannot be set."The correlation label is necessary for dedup/reopen/resolve flows (addressing the past review concern), but the comment should accurately reflect this limitation. Consider updating:
Suggested comment fix
- // Always add the internal correlation label so searchExistingIssue can - // find and dedup/reopen/resolve issues. User-defined labels are added - // only when explicitly configured, so Jira screens that do not include - // the Labels field are not affected when no labels are configured. + // Always add the internal correlation label so searchExistingIssue can + // find and dedup/reopen/resolve issues. User-defined labels are added + // only when explicitly configured. Note: the correlation label is always + // sent, so Jira projects/screens that do not include the Labels field + // cannot be used with this integration.The PR description should also be updated to clarify this limitation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@notify/jira/jira.go` around lines 195 - 215, The comment about "Jira screens that do not include the Labels field are not affected when no labels are configured" is inaccurate because the else branch sets requestBody.Fields.Labels = []string{alertLabel}, which makes Labels non-nil and causes MarshalJSON (types.go) to emit "labels" in the payload; update the comment near the requestBody.Fields.Labels logic to explicitly state that we always include the internal correlation label when no user labels are configured and that this will cause the "labels" field to be sent (and may fail for Jira screens that do not have Labels), and also update the PR description to mention this limitation; reference requestBody.Fields.Labels, n.conf.Labels, alertLabel and MarshalJSON in types.go so the reader can find the relevant code.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@notify/jira/jira.go`:
- Around line 195-215: The comment about "Jira screens that do not include the
Labels field are not affected when no labels are configured" is inaccurate
because the else branch sets requestBody.Fields.Labels = []string{alertLabel},
which makes Labels non-nil and causes MarshalJSON (types.go) to emit "labels" in
the payload; update the comment near the requestBody.Fields.Labels logic to
explicitly state that we always include the internal correlation label when no
user labels are configured and that this will cause the "labels" field to be
sent (and may fail for Jira screens that do not have Labels), and also update
the PR description to mention this limitation; reference
requestBody.Fields.Labels, n.conf.Labels, alertLabel and MarshalJSON in types.go
so the reader can find the relevant code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 4ef43bfa-bc2a-4de7-a4b7-65e6cde444bc
📒 Files selected for processing (1)
notify/jira/jira.go
1 participant
Fixes #5230
The Jira integration always sent a labels field in the issue creation request, even when no labels were configured. This caused failures on Jira project screens that do not include the Labels field, with the error Field 'labels' cannot be set.
Root cause: The issueFields struct was initialized with Labels: make([]string, 0, len(n.conf.Labels)+1), a non-nil empty slice, which serializes to "labels": [] in JSON and triggers the Jira validation error.
Fix: Initialize Labels as nil so it is omitted from the JSON payload when not configured. The existing if n.conf.Labels != nil guard then correctly only populates the field when the user explicitly configures labels.
Summary by CodeRabbit