Merge pull request #20323 from sonikro/fix-github-issues-hostname

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

.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

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(

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<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,

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',
   },

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<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 +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(() => {

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<string>,
+  repos: Array<Repository>,
   itemsPerRepo: number,
   options?: GithubIssuesByRepoOptions,
 ) => {