diff --git a/.changeset/two-terms-play.md b/.changeset/two-terms-play.md new file mode 100644 index 0000000000..6c7edc01db --- /dev/null +++ b/.changeset/two-terms-play.md @@ -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 diff --git a/plugins/github-issues/src/api/githubIssuesApi.test.ts b/plugins/github-issues/src/api/githubIssuesApi.test.ts index e960d3123b..f3c1b29675 100644 --- a/plugins/github-issues/src/api/githubIssuesApi.test.ts +++ b/plugins/github-issues/src/api/githubIssuesApi.test.ts @@ -21,9 +21,22 @@ 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 +123,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 +152,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 +267,7 @@ describe('githubIssuesApi', () => { ); const data = await api.fetchIssuesByRepoFromGithub( - ['mrwolny/yo-yo', 'mrwolny/notfound'], + [entityRepository('mrwolny/yo-yo'), entityRepository('mrwolny/notfound')], 10, ); @@ -301,7 +337,7 @@ describe('githubIssuesApi', () => { ); const data = await api.fetchIssuesByRepoFromGithub( - ['mrwolny/notfound'], + [entityRepository('mrwolny/notfound')], 10, ); @@ -341,7 +377,10 @@ 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( diff --git a/plugins/github-issues/src/api/githubIssuesApi.ts b/plugins/github-issues/src/api/githubIssuesApi.ts index 32331ef6b1..d7b39989d8 100644 --- a/plugins/github-issues/src/api/githubIssuesApi.ts +++ b/plugins/github-issues/src/api/githubIssuesApi.ts @@ -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, + repos: Array, itemsPerRepo: number, { filterBy, @@ -140,30 +146,35 @@ export const githubIssuesApi = ( }, }: GithubIssuesByRepoOptions = {}, ): Promise => { - const graphql = await getOctokit(); + const { graphql, hostname } = await getOctokit(); const safeNames: Array = []; + 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, diff --git a/plugins/github-issues/src/components/GithubIssues/GithubIssues.test.tsx b/plugins/github-issues/src/components/GithubIssues/GithubIssues.test.tsx index 3f9e829de9..bf3cac7423 100644 --- a/plugins/github-issues/src/components/GithubIssues/GithubIssues.test.tsx +++ b/plugins/github-issues/src/components/GithubIssues/GithubIssues.test.tsx @@ -68,6 +68,8 @@ const entityComponent = { metadata: { annotations: { 'github.com/project-slug': 'backstage/backstage', + 'backstage.io/source-location': + 'url:https://github.com/backstage/backstage', }, name: 'backstage', }, diff --git a/plugins/github-issues/src/hooks/useEntityGithubRepositories.ts b/plugins/github-issues/src/hooks/useEntityGithubRepositories.ts index f3fab72090..b9763298e6 100644 --- a/plugins/github-issues/src/hooks/useEntityGithubRepositories.ts +++ b/plugins/github-issues/src/hooks/useEntityGithubRepositories.ts @@ -14,10 +14,15 @@ * 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 +30,28 @@ 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([]); + const [repositories, setRepositories] = useState([]); 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 +64,26 @@ 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(() => { diff --git a/plugins/github-issues/src/hooks/useGetIssuesByRepoFromGithub.ts b/plugins/github-issues/src/hooks/useGetIssuesByRepoFromGithub.ts index 8ec5f3ec65..0b5bad7f57 100644 --- a/plugins/github-issues/src/hooks/useGetIssuesByRepoFromGithub.ts +++ b/plugins/github-issues/src/hooks/useGetIssuesByRepoFromGithub.ts @@ -16,10 +16,14 @@ 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, + repos: Array, itemsPerRepo: number, options?: GithubIssuesByRepoOptions, ) => {