paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(github-issues): fixes bug where entities that did not belong to the configured github instance were trying to be fetched

Browse Source 
Signed-off-by: Jonathan Nagayoshi <jonathan@nagayoshi.com.br>
This commit is contained in:
Jonathan Nagayoshi
2023-10-02 20:12:01 +00:00
parent 0507326be6
commit 7bd0a8ab3c
6 changed files with 98 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/two-terms-play.md
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/plugin-github-issues': patch
---
Filters out entities that belonged to a different github instance other than the one configured by the plugin
plugins/github-issues/src/api/githubIssuesApi.test.ts
+35 -7
View File
@@ -21,9 +21,14 @@ jest.mock('octokit', () => ({
import { ConfigApi, ErrorApi } from '@backstage/core-plugin-api';
import { ForwardedError } from '@backstage/errors';
import { createFilterByClause, githubIssuesApi } from './githubIssuesApi';
import type { GithubIssuesFilters } from './githubIssuesApi';
import { createFilterByClause, githubIssuesApi, Repository, GithubIssuesFilters } from './githubIssuesApi';
function entityRepository(name: string, locationHostname: string = "github.com"): Repository {
return {
locationHostname,
name
}
}
function getFragment(
filterBy = '',
orderBy = 'field: UPDATED_AT, direction: DESC',
@@ -110,7 +115,26 @@ describe('githubIssuesApi', () => {
it('should call GitHub API with correct query with fragment for each repo', async () => {
await api.fetchIssuesByRepoFromGithub(
['mrwolny/yo-yo', 'mrwolny/yoyo', 'mrwolny/yo.yo'],
[
entityRepository('mrwolny/yo-yo'),
entityRepository('mrwolny/yoyo'),
entityRepository('mrwolny/yo.yo'),
],
10,
);
expect(mockGraphQLQuery).toHaveBeenCalledTimes(1);
expect(mockGraphQLQuery).toHaveBeenCalledWith(getFragment());
});
it("should only fetch data for entities hosted in the same GitHub instance as the plugin's config", async () => {
await api.fetchIssuesByRepoFromGithub(
[
entityRepository('mrwolny/yo-yo'),
entityRepository('mrwolny/yoyo'),
entityRepository('mrwolny/yo.yo'),
entityRepository('mrwolny/another-repo', "enterprise.github.com"), // This one should be filtered out
],
10,
);
@@ -120,7 +144,11 @@ describe('githubIssuesApi', () => {
it('should call Github API with the correct filterBy and orderBy clauses', async () => {
await api.fetchIssuesByRepoFromGithub(
['mrwolny/yo-yo', 'mrwolny/yoyo', 'mrwolny/yo.yo'],
[
entityRepository('mrwolny/yo-yo'),
entityRepository('mrwolny/yoyo'),
entityRepository('mrwolny/yo.yo')
],
10,
{
filterBy: {
@@ -231,7 +259,7 @@ describe('githubIssuesApi', () => {
);
const data = await api.fetchIssuesByRepoFromGithub(
['mrwolny/yo-yo', 'mrwolny/notfound'],
[entityRepository('mrwolny/yo-yo'), entityRepository('mrwolny/notfound')],
10,
);
@@ -301,7 +329,7 @@ describe('githubIssuesApi', () => {
);
const data = await api.fetchIssuesByRepoFromGithub(
['mrwolny/notfound'],
[entityRepository('mrwolny/notfound')],
10,
);
@@ -341,7 +369,7 @@ describe('githubIssuesApi', () => {
mockErrorApi as unknown as ErrorApi,
);
await api.fetchIssuesByRepoFromGithub(['mrwolny/notfound'], 10);
await api.fetchIssuesByRepoFromGithub([entityRepository('mrwolny/notfound')], 10);
expect(mockErrorApi.post).toHaveBeenCalledTimes(1);
expect(mockErrorApi.post).toHaveBeenCalledWith(
plugins/github-issues/src/api/githubIssuesApi.ts
+33 -22
View File
@@ -24,6 +24,11 @@ import {
import { readGithubIntegrationConfigs } from '@backstage/integration';
import { ForwardedError } from '@backstage/errors';
/** @internal */
export type Repository = {
name: string;
locationHostname: string;
}
/** @internal */
export type Assignee = {
avatarUrl: string;
@@ -116,9 +121,10 @@ export const githubIssuesApi = (
let octokit: Octokit;
const getOctokit = async () => {
const baseUrl = readGithubIntegrationConfigs(
const githubConfig = readGithubIntegrationConfigs(
configApi.getOptionalConfigArray('integrations.github') ?? [],
)[0].apiBaseUrl;
)[0]
const baseUrl = githubConfig.apiBaseUrl;
const token = await githubAuthApi.getAccessToken(['repo']);
@@ -126,11 +132,11 @@ export const githubIssuesApi = (
octokit = new Octokit({ auth: token, ...(baseUrl && { baseUrl }) });
}
return octokit.graphql;
return { graphql: octokit.graphql, hostname: githubConfig.host };
};
const fetchIssuesByRepoFromGithub = async (
repos: Array<string>,
repos: Array<Repository>,
itemsPerRepo: number,
{
filterBy,
@@ -140,30 +146,35 @@ export const githubIssuesApi = (
},
}: GithubIssuesByRepoOptions = {},
): Promise<IssuesByRepo> => {
const graphql = await getOctokit();
const { graphql, hostname } = await getOctokit();
const safeNames: Array<string> = [];
const repositories = repos
// only tries to fetch issues from repositories that are hosted on the same GitHub instance as the octokit
.filter(repo => repo.locationHostname === hostname)
.map(repo => {
const [owner, name] = repo.name.split('/');
const repositories = repos.map(repo => {
const [owner, name] = repo.split('/');
const safeNameRegex = /-|\./gi;
let safeName = name.replace(safeNameRegex, '');
const safeNameRegex = /-|\./gi;
let safeName = name.replace(safeNameRegex, '');
while (safeNames.includes(safeName)) {
safeName += 'x';
}
while (safeNames.includes(safeName)) {
safeName += 'x';
}
safeNames.push(safeName);
safeNames.push(safeName);
return {
safeName,
name,
owner,
};
});
return {
safeName,
name,
owner,
};
});
let issuesByRepo: IssuesByRepo = {};
try {
if(repositories.length === 0) {
throw new Error(`No repositories found for ${hostname}`)
}
issuesByRepo = await graphql(
createIssueByRepoQuery(repositories, itemsPerRepo, {
filterBy,
@@ -273,12 +284,12 @@ function createIssueByRepoQuery(
query {
${repositories.map(
({ safeName, name, owner }) => `
({ safeName, name, owner }) => `
${safeName}: repository(name: "${name}", owner: "${owner}") {
...issues
}
`,
)}
)}
}
`;
plugins/github-issues/src/components/GithubIssues/GithubIssues.test.tsx
+1
View File
@@ -68,6 +68,7 @@ const entityComponent = {
metadata: {
annotations: {
'github.com/project-slug': 'backstage/backstage',
'backstage.io/source-location': "url:https://github.com/backstage/backstage"
},
name: 'backstage',
},
plugins/github-issues/src/hooks/useEntityGithubRepositories.ts
+22 -8
View File
@@ -14,10 +14,11 @@
* limitations under the License.
*/
import { Entity, stringifyEntityRef } from '@backstage/catalog-model';
import { Entity, stringifyEntityRef, getEntitySourceLocation } from '@backstage/catalog-model';
import { useApi } from '@backstage/core-plugin-api';
import { catalogApiRef, useEntity } from '@backstage/plugin-catalog-react';
import { useCallback, useEffect, useState } from 'react';
import { Repository } from '../api';
const GITHUB_PROJECT_SLUG_ANNOTATION = 'github.com/project-slug';
@@ -25,18 +26,26 @@ export const getProjectNameFromEntity = (entity: Entity): string => {
return entity?.metadata.annotations?.[GITHUB_PROJECT_SLUG_ANNOTATION] ?? '';
};
export const getHostnameFromEntity = (entity: Entity): string => {
const { target } = getEntitySourceLocation(entity)
return new URL(target).hostname
}
export function useEntityGithubRepositories() {
const { entity } = useEntity();
const catalogApi = useApi(catalogApiRef);
const [repositories, setRepositories] = useState<string[]>([]);
const [repositories, setRepositories] = useState<Repository[]>([]);
const getRepositoriesNames = useCallback(async () => {
if (entity.kind === 'Component' || entity.kind === 'API') {
const entityName = getProjectNameFromEntity(entity);
const locationHostname = getHostnameFromEntity(entity)
if (entityName) {
setRepositories([entityName]);
setRepositories([{
name: entityName,
locationHostname
}]);
}
return;
@@ -49,11 +58,16 @@ export function useEntityGithubRepositories() {
},
});
const entitiesNames: string[] = entitiesList.items.map(componentEntity =>
getProjectNameFromEntity(componentEntity),
);
const repositoryEntities: Repository[] = entitiesList.items.reduce((acc: Repository[], componentEntity: Entity) => {
const entityName = getProjectNameFromEntity(componentEntity);
const entityLocationHostname = getHostnameFromEntity(componentEntity);
if (entityName && !acc.some((it: Repository) => it.name === entityName) && entityName.length) {
acc.push({ name: entityName, locationHostname: entityLocationHostname });
}
return acc;
}, []);
setRepositories([...new Set(entitiesNames)].filter(name => name.length));
setRepositories(repositoryEntities);
}, [catalogApi, entity]);
useEffect(() => {
plugins/github-issues/src/hooks/useGetIssuesByRepoFromGithub.ts
+2 -2
View File
@@ -16,10 +16,10 @@
import { useApi } from '@backstage/core-plugin-api';
import useAsyncRetry from 'react-use/lib/useAsyncRetry';
import { githubIssuesApiRef, GithubIssuesByRepoOptions } from '../api';
import { Repository, githubIssuesApiRef, GithubIssuesByRepoOptions } from '../api';
export const useGetIssuesByRepoFromGithub = (
repos: Array<string>,
repos: Array<Repository>,
itemsPerRepo: number,
options?: GithubIssuesByRepoOptions,
) => {