Skip to content

Fix Scorecard SonarQube "Open Issues" metric pulling #4274

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

Closed
imykhno wants to merge 1 commit into from
Closed

Fix Scorecard SonarQube "Open Issues" metric pulling #4274

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
5 changes: 5 additions & 0 deletions workspaces/scorecard/.changeset/six-seas-wear.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@red-hat-developer-hub/backstage-plugin-scorecard-backend-module-sonarqube': patch
---

Resolve issue for `sonarqube.openIssues` Scorecard SonarQube metric when the project is inaccessible.
80 changes: 74 additions & 6 deletions .../scorecard/plugins/scorecard-backend-module-sonarqube/src/clients/SonarQubeClient.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,20 +83,88 @@ describe('SonarQubeClient', () => {
});

describe('getOpenIssuesCount', () => {
it('returns the total count of open issues', async () => {
mockFetch.mockResolvedValueOnce({
ok: true,
json: async () => ({ total: 42 }),
});
it('returns the total count of open issues after verifying project access', async () => {
mockFetch
.mockResolvedValueOnce({
ok: true,
json: async () => ({ component: { key: 'my-project' } }),
})
.mockResolvedValueOnce({
ok: true,
json: async () => ({
total: 42,
paging: { pageIndex: 1, pageSize: 1, total: 42 },
}),
});

const result = await client.getOpenIssuesCount('my-project');

expect(result).toBe(42);
expect(mockFetch).toHaveBeenCalledWith(
expect(mockFetch).toHaveBeenNthCalledWith(
1,
'https://sonarcloud.io/api/components/show?component=my-project',
expect.any(Object),
);
expect(mockFetch).toHaveBeenNthCalledWith(
2,
'https://sonarcloud.io/api/issues/search?componentKeys=my-project&statuses=OPEN,CONFIRMED,REOPENED&ps=1',
expect.any(Object),
);
});

@fullsend-ai-review fullsend-ai-review Bot Aug 12, 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.

[low] test adequacy

The test throws when project access check fails only tests the HTTP 404 case. There is no test covering the scenario where an invalid instanceName is passed to getOpenIssuesCount — this would reveal the regression where the configuration error (instance not found) is now incorrectly masked as project not accessible.


it('throws when project access check fails and does not search issues', async () => {
mockFetch.mockResolvedValueOnce({
ok: false,
status: 404,
statusText: 'Not Found',
});

await expect(client.getOpenIssuesCount('my-project')).rejects.toThrow(
"SonarQube project 'my-project' is not accessible or the project key is missing or invalid",
);
expect(mockFetch).toHaveBeenCalledTimes(1);
expect(mockFetch).toHaveBeenCalledWith(
'https://sonarcloud.io/api/components/show?component=my-project',
expect.any(Object),
);
});

it('propagates API errors from issues search after access check succeeds', async () => {
mockFetch
.mockResolvedValueOnce({
ok: true,
json: async () => ({ component: { key: 'my-project' } }),
})
.mockResolvedValueOnce({
ok: false,
status: 503,
statusText: 'Service Unavailable',
});

await expect(client.getOpenIssuesCount('my-project')).rejects.toThrow(
/SonarQube API error: 503 Service Unavailable/,
);
expect(mockFetch).toHaveBeenCalledTimes(2);
});

it('returns 0 when the project is accessible and has no open issues', async () => {
mockFetch
.mockResolvedValueOnce({
ok: true,
json: async () => ({ component: { key: 'my-project' } }),
})
.mockResolvedValueOnce({
ok: true,
json: async () => ({
total: 0,
paging: { pageIndex: 1, pageSize: 1, total: 0 },
}),
});

const result = await client.getOpenIssuesCount('my-project');

expect(result).toBe(0);
});
});

describe('getMeasures', () => {
Expand Down
31 changes: 23 additions & 8 deletions ...paces/scorecard/plugins/scorecard-backend-module-sonarqube/src/clients/SonarQubeClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,29 @@ export class SonarQubeClient {
projectKey: string,
instanceName?: string,
): Promise<number> {
this.logger.debug(`Fetching open issues count for project ${projectKey}`);
const data = await this.fetchApi(
`/api/issues/search?componentKeys=${encodeURIComponent(
projectKey,
)}&statuses=OPEN,CONFIRMED,REOPENED&ps=1`,
instanceName,
);
return data.total;
this.logger.debug(`Fetching open issues count for project ${projectKey}`);

// Additional check to ensure the project is accessible
try {

@fullsend-ai-review fullsend-ai-review Bot Aug 12, 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.

[low] indentation

The method body of getOpenIssuesCount is indented at 6 spaces, while every other method in this class uses 4 spaces. This inconsistency was introduced by the diff.

Suggested fix: Re-indent the entire getOpenIssuesCount method body to use 4-space indentation, matching the sibling methods.

@fullsend-ai-review fullsend-ai-review Bot Aug 12, 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.

[low] intent-clarity

The pre-flight call to /api/components/show runs on every invocation, adding latency and load to the happy path. The changeset describes fixing an issue when the project is inaccessible but the implementation adds overhead to all calls.

await this.fetchApi(
`/api/components/show?component=${encodeURIComponent(projectKey)}`,
instanceName,
);
}
catch {
throw new Error(
`SonarQube project '${projectKey}' is not accessible or the project key is missing or invalid`,

@fullsend-ai-review fullsend-ai-review Bot Aug 12, 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.

[medium] error handling

The bare catch block around the project-access check swallows all errors indiscriminately and replaces them with a misleading project is not accessible message. This catches not only HTTP 4xx responses (the intended case) but also: (1) configuration errors from resolveInstance (e.g., SonarQube instance unknown not found in configuration would be masked), (2) network failures, (3) server errors (HTTP 500/503). For case (1), this is a behavioral regression — before this change, passing an invalid instanceName would throw a clear configuration error; now it throws a misleading project not accessible error.

Suggested fix: Narrow the catch to only handle the fetchApi HTTP error case. Check if (error instanceof Error && error.message.includes(SonarQube API error)) and only rethrow the project-accessibility message for that case, re-throwing the original error otherwise.

);
}

@fullsend-ai-review fullsend-ai-review Bot Aug 12, 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.

[low] brace placement

The catch keyword is placed on a new line after the closing brace. The standard JavaScript/TypeScript convention (and what prettier enforces) is same-line style (} catch {).

Suggested fix: Move catch onto the same line as the closing brace: } catch {.


const data = await this.fetchApi(
`/api/issues/search?componentKeys=${encodeURIComponent(
projectKey,
)}&statuses=OPEN,CONFIRMED,REOPENED&ps=1`,
instanceName,
);

return data.paging?.total ?? data.total ?? 0;

@fullsend-ai-review fullsend-ai-review Bot Aug 12, 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.

[medium] scope-creep

The claimed intent is to handle inaccessible projects, but the change also silently alters return-value parsing from data.total to data.paging?.total ?? data.total ?? 0. This is a separate behavioral change not mentioned in the PR title, body, or changeset description.

Suggested fix: Either revert the return expression to data.total if the pre-flight check alone solves the bug, or document why the parsing change was needed.

}

@fullsend-ai-review fullsend-ai-review Bot Aug 12, 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.

[low] error handling / silent fallback

data.paging?.total ?? data.total ?? 0 silently returns 0 when neither paging.total nor total is present in the API response. If the SonarQube API response structure changes unexpectedly, this will produce a silent 0 rather than a visible failure.


async getMeasures(
Expand Down
Loading