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..f965458858 100644 --- a/plugins/github-issues/src/api/githubIssuesApi.test.ts +++ b/plugins/github-issues/src/api/githubIssuesApi.test.ts @@ -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( diff --git a/plugins/github-issues/src/api/githubIssuesApi.ts b/plugins/github-issues/src/api/githubIssuesApi.ts index 32331ef6b1..888d3c4c64 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, @@ -273,12 +284,12 @@ function createIssueByRepoQuery( query { ${repositories.map( - ({ safeName, name, owner }) => ` + ({ safeName, name, owner }) => ` ${safeName}: repository(name: "${name}", owner: "${owner}") { ...issues } `, - )} + )} } `; diff --git a/plugins/github-issues/src/components/GithubIssues/GithubIssues.test.tsx b/plugins/github-issues/src/components/GithubIssues/GithubIssues.test.tsx index 3f9e829de9..8123616721 100644 --- a/plugins/github-issues/src/components/GithubIssues/GithubIssues.test.tsx +++ b/plugins/github-issues/src/components/GithubIssues/GithubIssues.test.tsx @@ -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', }, diff --git a/plugins/github-issues/src/hooks/useEntityGithubRepositories.ts b/plugins/github-issues/src/hooks/useEntityGithubRepositories.ts index f3fab72090..b8e02c2109 100644 --- a/plugins/github-issues/src/hooks/useEntityGithubRepositories.ts +++ b/plugins/github-issues/src/hooks/useEntityGithubRepositories.ts @@ -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([]); + 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 +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(() => { diff --git a/plugins/github-issues/src/hooks/useGetIssuesByRepoFromGithub.ts b/plugins/github-issues/src/hooks/useGetIssuesByRepoFromGithub.ts index 8ec5f3ec65..6891655f61 100644 --- a/plugins/github-issues/src/hooks/useGetIssuesByRepoFromGithub.ts +++ b/plugins/github-issues/src/hooks/useGetIssuesByRepoFromGithub.ts @@ -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, + repos: Array, itemsPerRepo: number, options?: GithubIssuesByRepoOptions, ) => {