Skip to content

Fix Jira integration for Atlassian Cloud migration #916

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.

Sign up for GitHub

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

Merged
openshift-merge-bot merged 1 commit into from
Apr 12, 2026
Merged

Fix Jira integration for Atlassian Cloud migration #916

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion pkg/cli/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ type BackplaneConfiguration struct {
PagerDutyAPIKey string `json:"pd-key"`
JiraBaseURL string `json:"jira-base-url"`
JiraToken string `json:"jira-token"`
JiraEmail string `json:"jira-email"`
JiraConfigForAccessRequests AccessRequestsJiraConfiguration `json:"jira-config-for-access-requests"`
VPNCheckEndpoint string `json:"vpn-check-endpoint"`
ProxyCheckEndpoint string `json:"proxy-check-endpoint"`
Expand All @@ -58,9 +59,10 @@ const (
JiraBaseURLKey = "jira-base-url"
AssumeInitialArnKey = "assume-initial-arn"
JiraTokenViperKey = "jira-token"
JiraEmailViperKey = "jira-email"
JiraConfigForAccessRequestsKey = "jira-config-for-access-requests"
prodEnvNameDefaultValue = "production"
JiraBaseURLDefaultValue = "https://issues.redhat.com"
JiraBaseURLDefaultValue = "https://redhat.atlassian.net"

@coderabbitai coderabbitai Bot Apr 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Default domain switch exposes domain-coupled issue ID parsing in access requests.

After Line 65 changes the default to .net, pkg/accessrequest/accessRequest.go (Line 82) still strips URLs using strings.TrimPrefix(issueID, getJiraBaseURL()+"/browse/"). Full legacy .com browse URLs won’t be normalized and can fail later issue-key handling. Consider making issue-ID extraction domain-agnostic (parse /browse/<KEY> from any Jira host).

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pkg/cli/config/config.go` at line 65, The current normalization in
pkg/accessrequest/accessRequest.go relies on getJiraBaseURL() (and the constant
JiraBaseURLDefaultValue) to TrimPrefix(issueID, getJiraBaseURL()+"/browse/"),
which breaks when Jira hosts differ (.com vs .net); change the extraction to be
domain-agnostic by locating the "/browse/" path segment (or using a regex to
capture the segment after "/browse/") and extracting the issue key regardless of
host, updating the logic in the function that performs this normalization so it
no longer depends on getJiraBaseURL() or the JiraBaseURLDefaultValue.

proxyTestTimeout = 10 * time.Second
GovcloudDefaultValue bool = false
GovcloudDefaultValueKey = "govcloud"
Expand Down Expand Up @@ -232,6 +234,13 @@ func GetBackplaneConfigurationWithConn(ocmConn *ocmsdk.Connection) (bpConfig Bac
bpConfig.JiraToken = viper.GetString(JiraTokenViperKey)
}

// JIRA email is required for Atlassian Cloud basic auth
// JIRA_EMAIL env var takes precedence, fallback to config file
bpConfig.JiraEmail = os.Getenv(info.BackplaneJiraEmailEnvName)
if bpConfig.JiraEmail == "" {
bpConfig.JiraEmail = viper.GetString(JiraEmailViperKey)
}

// JIRA config for access requests is optional as there is a default value
err = viper.UnmarshalKey(JiraConfigForAccessRequestsKey, &bpConfig.JiraConfigForAccessRequests)

Expand Down
80 changes: 80 additions & 0 deletions pkg/cli/config/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,86 @@ func TestGetBackplaneConfig(t *testing.T) {
}
})

t.Run("it reads JIRA email from JIRA_EMAIL environment variable when config file is empty", func(t *testing.T) {
viper.Reset()
svr := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
_, _ = w.Write([]byte("dummy data"))
}))

expectedEmail := "user@redhat.com"
userDefinedProxy := "example-proxy"
t.Setenv("BACKPLANE_URL", svr.URL)
t.Setenv("HTTPS_PROXY", userDefinedProxy)
t.Setenv("JIRA_EMAIL", expectedEmail)

config, err := GetBackplaneConfiguration()
if err != nil {
t.Error(err)
}

if config.JiraEmail != expectedEmail {
t.Errorf("expected JiraEmail to be %s, got %s", expectedEmail, config.JiraEmail)
}
})

t.Run("JIRA_EMAIL environment variable takes precedence over config file value", func(t *testing.T) {
viper.Reset()
svr := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
_, _ = w.Write([]byte("dummy data"))
}))

configEmail := "config@redhat.com"
envEmail := "env@redhat.com"

t.Setenv("BACKPLANE_URL", svr.URL)
t.Setenv("JIRA_EMAIL", envEmail)

// Simulate config file value
viper.Set(JiraEmailViperKey, configEmail)

config, err := GetBackplaneConfiguration()
if err != nil {
t.Error(err)
}

if config.JiraEmail != envEmail {
t.Errorf("expected environment variable email to take precedence: expected %s, got %s", envEmail, config.JiraEmail)
}
})

t.Run("JiraEmail falls back to config file when JIRA_EMAIL env var is not set", func(t *testing.T) {
viper.Reset()

tmpDir := t.TempDir()
configPath := tmpDir + "/config.json"
configContent := `{
"jira-email": "configfile@redhat.com"
}`
err := os.WriteFile(configPath, []byte(configContent), 0600)
if err != nil {
t.Fatal(err)
}

t.Setenv("BACKPLANE_CONFIG", configPath)

svr := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
_, _ = w.Write([]byte("dummy data"))
}))
defer svr.Close()

t.Setenv("BACKPLANE_URL", svr.URL)
t.Setenv("HTTPS_PROXY", "example-proxy")

config, err := GetBackplaneConfiguration()
if err != nil {
t.Error(err)
}

if config.JiraEmail != "configfile@redhat.com" {
t.Errorf("expected JiraEmail from config file, got %s", config.JiraEmail)
}
})

t.Run("JIRA_API_TOKEN environment variable takes precedence over config file JIRA token", func(t *testing.T) {
viper.Reset()
svr := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
Expand Down
1 change: 1 addition & 0 deletions pkg/info/info.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const (
BackplaneConfigPathEnvName = "BACKPLANE_CONFIG"
BackplaneKubeconfigEnvName = "KUBECONFIG"
BackplaneJiraAPITokenEnvName = "JIRA_API_TOKEN" //nolint:gosec
BackplaneJiraEmailEnvName = "JIRA_EMAIL"

// Configuration
BackplaneConfigDefaultFilePath = ".config/backplane"
Expand Down
11 changes: 8 additions & 3 deletions pkg/jira/issueService.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,16 @@ func createIssueService() (*jira.IssueService, error) {
}

if bpConfig.JiraToken == "" {
return nil, fmt.Errorf("JIRA token is not defined, consider defining it running 'ocm-backplane config set %s <token value>'", config.JiraTokenViperKey)
return nil, fmt.Errorf("JIRA API token is not defined, consider defining it running 'ocm-backplane config set %s <token value>'", config.JiraTokenViperKey)
}

transport := jira.PATAuthTransport{
Token: bpConfig.JiraToken,
if bpConfig.JiraEmail == "" {
return nil, fmt.Errorf("JIRA email is not defined, consider defining it running 'ocm-backplane config set %s <email>' or setting the JIRA_EMAIL environment variable", config.JiraEmailViperKey)
}

transport := jira.BasicAuthTransport{
Username: bpConfig.JiraEmail,
Password: bpConfig.JiraToken,
}

jiraClient, err := jira.NewClient(transport.Client(), bpConfig.JiraBaseURL)
Expand Down
7 changes: 4 additions & 3 deletions pkg/jira/ohssService.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package jira

import (
"fmt"
"strings"
"net/url"

"github.com/andygrunwald/go-jira"
)
Expand Down Expand Up @@ -72,8 +72,9 @@ func (j *OHSSService) formatIssue(issue jira.Issue) (formatIssue OHSSIssue, err
formatIssue.Title = issue.Fields.Summary
}
if issue.Self != "" {
selfSlice := strings.SplitAfter(issue.Self, ".com")
formatIssue.WebURL = fmt.Sprintf("%s/browse/%s", selfSlice[0], issue.Key)
if u, err := url.Parse(issue.Self); err == nil {
formatIssue.WebURL = fmt.Sprintf("%s://%s/browse/%s", u.Scheme, u.Host, issue.Key)
}
Comment on lines +75 to +77

@coderabbitai coderabbitai Bot Apr 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Current WebURL parsing logic =="
rg -n -C3 'url\.Parse\(issue\.Self\)|WebURL' pkg/jira/ohssService.go

echo
echo "== Existing Self URL test cases =="
rg -n -C2 'Self\s*=' pkg/jira/ohssService_test.go

echo
echo "== Check for malformed/relative Self coverage =="
rg -n -C2 'malformed|invalid|relative|scheme|host' pkg/jira/ohssService_test.go || true

Repository: openshift/backplane-cli

Length of output: 1321

Harden WebURL construction by validating parsed URL components.

At lines 75-77, url.Parse() succeeds for relative URLs without returning an error, but leaves u.Scheme and u.Host empty. This produces malformed WebURLs like :///browse/<key>. Add explicit checks for HTTP(S) scheme and non-empty host.

Current code has no test coverage for relative or malformed Self values.

Proposed fix 
-		if u, err := url.Parse(issue.Self); err == nil {
+		if u, err := url.Parse(issue.Self); err == nil &&
+			(u.Scheme == "http" || u.Scheme == "https") &&
+			u.Host != "" {
 			formatIssue.WebURL = fmt.Sprintf("%s://%s/browse/%s", u.Scheme, u.Host, issue.Key)
 		}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if u, err := url.Parse(issue.Self); err == nil {
formatIssue.WebURL = fmt.Sprintf("%s://%s/browse/%s", u.Scheme, u.Host, issue.Key)
}
if u, err := url.Parse(issue.Self); err == nil &&
(u.Scheme == "http" || u.Scheme == "https") &&
u.Host != "" {
formatIssue.WebURL = fmt.Sprintf("%s://%s/browse/%s", u.Scheme, u.Host, issue.Key)
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pkg/jira/ohssService.go` around lines 75 - 77, The WebURL construction using
url.Parse(issue.Self) can produce malformed values when Parse returns a relative
URL (empty Scheme/Host); update the logic around url.Parse(issue.Self) (the
block that sets formatIssue.WebURL) to validate that u.Scheme is "http" or
"https" and u.Host is non-empty before formatting "%s://%s/browse/%s"; if those
checks fail, do not build the WebURL (leave it empty or use a safe fallback),
and add unit tests for relative/malformed issue.Self inputs to cover this case.

}

return formatIssue, nil
Expand Down
42 changes: 42 additions & 0 deletions pkg/jira/ohssService_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,5 +62,47 @@ var _ = Describe("Jira", func() {
Expect(err).NotTo(BeNil())
Expect(err.Error()).To(Equal("no matching issue for issueID:OHSS-1000"))
})

It("Should parse Atlassian Cloud .net Self URL into WebURL", func() {
testIssue.Key = testOHSSID
testIssue.Self = "https://redhat.atlassian.net/rest/api/2/issue/12345"
mockIssueService.EXPECT().Get(testOHSSID, nil).Return(&testIssue, nil, nil).Times(1)

issue, err := ohssService.GetIssue(testOHSSID)
Expect(err).To(BeNil())
Expect(issue.WebURL).To(Equal("https://redhat.atlassian.net/browse/OHSS-1000"))
})

It("Should parse legacy .com Self URL into WebURL", func() {
testIssue.Key = testOHSSID
testIssue.Self = "https://issues.redhat.com/rest/api/2/issue/12345"
mockIssueService.EXPECT().Get(testOHSSID, nil).Return(&testIssue, nil, nil).Times(1)

issue, err := ohssService.GetIssue(testOHSSID)
Expect(err).To(BeNil())
Expect(issue.WebURL).To(Equal("https://issues.redhat.com/browse/OHSS-1000"))
})

It("Should return empty WebURL when Self is empty", func() {
testIssue.Key = testOHSSID
testIssue.Self = ""
mockIssueService.EXPECT().Get(testOHSSID, nil).Return(&testIssue, nil, nil).Times(1)

issue, err := ohssService.GetIssue(testOHSSID)
Expect(err).To(BeNil())
Expect(issue.WebURL).To(Equal(""))
})

It("Should extract ClusterID from custom field", func() {
issueFields.Unknowns = map[string]interface{}{
CustomFieldClusterID: "abc-123-def-456",
}
testIssue.Fields = issueFields
mockIssueService.EXPECT().Get(testOHSSID, nil).Return(&testIssue, nil, nil).Times(1)

issue, err := ohssService.GetIssue(testOHSSID)
Expect(err).To(BeNil())
Expect(issue.ClusterID).To(Equal("abc-123-def-456"))
})
})
})