Merge pull request #15953 from costasd/costasd/filter_forks

GithubDiscoveryProcessor: allow filtering on forks
This commit is contained in:
Ben Lambert
2023-01-27 10:27:42 +01:00
committed by GitHub
11 changed files with 115 additions and 4 deletions
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/plugin-catalog-backend-module-github': patch
---
Add support for filtering out forks
+8
View File
@@ -86,6 +86,10 @@ export interface Config {
/**
* (Optional) GitHub topic-based filters.
*/
allowForks?: boolean;
/**
* (Optional) Allow Forks to be evaluated.
*/
topic?: {
/**
* (Optional) An array of strings used to filter in results based on their associated GitHub topics.
@@ -143,6 +147,10 @@ export interface Config {
/**
* (Optional) GitHub topic-based filters.
*/
allowForks?: boolean;
/**
* (Optional) Allow Forks to be evaluated.
*/
topic?: {
/**
* (Optional) An array of strings used to filter in results based on their associated GitHub topics.
@@ -488,6 +488,7 @@ describe('github', () => {
name: 'backstage',
url: 'https://github.com/backstage/backstage',
isArchived: false,
isFork: false,
repositoryTopics: {
nodes: [{ topic: { name: 'blah' } }],
},
@@ -500,6 +501,7 @@ describe('github', () => {
name: 'demo',
url: 'https://github.com/backstage/demo',
isArchived: true,
isFork: true,
repositoryTopics: { nodes: [] },
defaultBranchRef: {
name: 'main',
@@ -524,6 +526,7 @@ describe('github', () => {
name: 'backstage',
url: 'https://github.com/backstage/backstage',
isArchived: false,
isFork: false,
repositoryTopics: {
nodes: [{ topic: { name: 'blah' } }],
},
@@ -536,6 +539,7 @@ describe('github', () => {
name: 'demo',
url: 'https://github.com/backstage/demo',
isArchived: true,
isFork: true,
repositoryTopics: { nodes: [] },
defaultBranchRef: {
name: 'main',
@@ -89,6 +89,7 @@ export type RepositoryResponse = {
name: string;
url: string;
isArchived: boolean;
isFork: boolean;
repositoryTopics: RepositoryTopics;
defaultBranchRef: {
name: string;
@@ -435,6 +436,7 @@ export async function getOrganizationRepositories(
}
url
isArchived
isFork
repositoryTopics(first: 100) {
nodes {
... on RepositoryTopic {
@@ -15,7 +15,11 @@
*/
import { GithubTopicFilters } from '../providers/GithubEntityProviderConfig';
import { parseGithubOrgUrl, satisfiesTopicFilter } from './util';
import {
parseGithubOrgUrl,
satisfiesTopicFilter,
satisfiesForkFilter,
} from './util';
describe('parseGithubOrgUrl', () => {
it('only supports clean org urls, and decodes them', () => {
@@ -87,3 +91,21 @@ describe('satisfiesTopicFilter', () => {
).toEqual(false);
});
});
describe('satisfiesForkFilter', () => {
it('handles cases where forks are not allowed and a fork is evaluated', () => {
expect(satisfiesForkFilter(false, true)).toEqual(false);
});
it('handles cases where forks are not allowed and a fork is not evaluated', () => {
expect(satisfiesForkFilter(false, false)).toEqual(true);
});
it('handles cases where forks are allowed and a fork is evaluated', () => {
expect(satisfiesForkFilter(true, true)).toEqual(true);
});
it('handles cases where forks are allowed and a fork is not evaluated', () => {
expect(satisfiesForkFilter(true, false)).toEqual(true);
});
});
@@ -77,3 +77,14 @@ export function satisfiesTopicFilter(
// not configured at all.
return true;
}
export function satisfiesForkFilter(
allowForks: boolean | true,
isFork: boolean | false,
): Boolean {
// we don't want to include forks if forks are not allowed
if (!allowForks && isFork) return false;
// if forks are allowed, allow all repos through
return true;
}
@@ -150,6 +150,7 @@ describe('GithubDiscoveryProcessor', () => {
url: 'https://github.com/backstage/backstage',
repositoryTopics: { nodes: [] },
isArchived: false,
isFork: false,
defaultBranchRef: {
name: 'master',
},
@@ -160,6 +161,7 @@ describe('GithubDiscoveryProcessor', () => {
url: 'https://github.com/backstage/demo',
repositoryTopics: { nodes: [] },
isArchived: false,
isFork: false,
defaultBranchRef: {
name: 'main',
},
@@ -202,6 +204,7 @@ describe('GithubDiscoveryProcessor', () => {
url: 'https://github.com/backstage/tech-docs',
repositoryTopics: { nodes: [] },
isArchived: false,
isFork: false,
defaultBranchRef: {
name: 'main',
},
@@ -236,6 +239,7 @@ describe('GithubDiscoveryProcessor', () => {
url: 'https://github.com/backstage/tech-docs',
repositoryTopics: { nodes: [] },
isArchived: false,
isFork: false,
defaultBranchRef: null,
catalogInfoFile: null,
},
@@ -260,6 +264,7 @@ describe('GithubDiscoveryProcessor', () => {
url: 'https://github.com/backstage/backstage',
repositoryTopics: { nodes: [] },
isArchived: false,
isFork: false,
defaultBranchRef: {
name: 'master',
},
@@ -295,6 +300,7 @@ describe('GithubDiscoveryProcessor', () => {
url: 'https://github.com/backstage/backstage',
repositoryTopics: { nodes: [] },
isArchived: false,
isFork: false,
defaultBranchRef: {
name: 'main',
},
@@ -305,6 +311,7 @@ describe('GithubDiscoveryProcessor', () => {
url: 'https://github.com/backstage/techdocs-cli',
repositoryTopics: { nodes: [] },
isArchived: false,
isFork: false,
defaultBranchRef: {
name: 'main',
},
@@ -315,6 +322,7 @@ describe('GithubDiscoveryProcessor', () => {
url: 'https://github.com/backstage/techdocs-container',
repositoryTopics: { nodes: [] },
isArchived: false,
isFork: false,
defaultBranchRef: {
name: 'main',
},
@@ -325,6 +333,7 @@ describe('GithubDiscoveryProcessor', () => {
url: 'https://github.com/backstage/techdocs-durp',
repositoryTopics: { nodes: [] },
isArchived: false,
isFork: false,
defaultBranchRef: null,
catalogInfoFile: null,
},
@@ -365,6 +374,7 @@ describe('GithubDiscoveryProcessor', () => {
name: 'abstest',
url: 'https://github.com/backstage/abctest',
isArchived: false,
isFork: false,
repositoryTopics: { nodes: [] },
defaultBranchRef: {
name: 'main',
@@ -375,6 +385,7 @@ describe('GithubDiscoveryProcessor', () => {
name: 'test',
url: 'https://github.com/backstage/test',
isArchived: false,
isFork: false,
repositoryTopics: { nodes: [] },
defaultBranchRef: {
name: 'main',
@@ -386,6 +397,7 @@ describe('GithubDiscoveryProcessor', () => {
url: 'https://github.com/backstage/test',
repositoryTopics: { nodes: [] },
isArchived: true,
isFork: false,
defaultBranchRef: {
name: 'main',
},
@@ -396,6 +408,7 @@ describe('GithubDiscoveryProcessor', () => {
url: 'https://github.com/backstage/testxyz',
repositoryTopics: { nodes: [] },
isArchived: false,
isFork: false,
defaultBranchRef: {
name: 'main',
},
@@ -172,6 +172,7 @@ describe('GithubEntityProvider', () => {
url: 'https://github.com/test-org/test-repo',
repositoryTopics: { nodes: [] },
isArchived: false,
isFork: false,
defaultBranchRef: {
name: 'main',
},
@@ -274,6 +275,7 @@ describe('GithubEntityProvider', () => {
],
},
isArchived: false,
isFork: false,
defaultBranchRef: {
name: 'main',
},
@@ -353,6 +355,7 @@ describe('GithubEntityProvider', () => {
],
},
isArchived: false,
isFork: false,
defaultBranchRef: {
name: 'main',
},
@@ -445,6 +448,7 @@ describe('GithubEntityProvider', () => {
nodes: [],
},
isArchived: false,
isFork: false,
defaultBranchRef: {
name: 'main',
},
@@ -457,6 +461,7 @@ describe('GithubEntityProvider', () => {
nodes: [],
},
isArchived: false,
isFork: false,
defaultBranchRef: {
name: 'main',
},
@@ -557,6 +562,7 @@ describe('GithubEntityProvider', () => {
],
},
isArchived: false,
isFork: false,
defaultBranchRef: {
name: 'main',
},
@@ -580,6 +586,7 @@ describe('GithubEntityProvider', () => {
],
},
isArchived: false,
isFork: false,
defaultBranchRef: {
name: 'main',
},
@@ -600,6 +607,7 @@ describe('GithubEntityProvider', () => {
],
},
isArchived: false,
isFork: false,
defaultBranchRef: {
name: 'main',
},
@@ -40,7 +40,7 @@ import {
GithubEntityProviderConfig,
} from './GithubEntityProviderConfig';
import { getOrganizationRepositories } from '../lib/github';
import { satisfiesTopicFilter } from '../lib/util';
import { satisfiesTopicFilter, satisfiesForkFilter } from '../lib/util';
import { EventParams, EventSubscriber } from '@backstage/plugin-events-node';
import { PushEvent, Commit } from '@octokit/webhooks-types';
@@ -52,6 +52,7 @@ type Repository = {
name: string;
url: string;
isArchived: boolean;
isFork: boolean;
repositoryTopics: string[];
defaultBranchRef?: string;
isCatalogInfoFilePresent: boolean;
@@ -216,6 +217,7 @@ export class GithubEntityProvider implements EntityProvider, EventSubscriber {
defaultBranchRef: r.defaultBranchRef?.name,
repositoryTopics: r.repositoryTopics.nodes.map(t => t.topic.name),
isArchived: r.isArchived,
isFork: r.isFork,
isCatalogInfoFilePresent:
r.catalogInfoFile?.__typename === 'Blob' &&
r.catalogInfoFile.text !== '',
@@ -234,6 +236,7 @@ export class GithubEntityProvider implements EntityProvider, EventSubscriber {
private matchesFilters(repositories: Repository[]) {
const repositoryFilter = this.config.filters?.repository;
const topicFilters = this.config.filters?.topic;
const allowForks = this.config.filters?.allowForks ?? true;
const matchingRepositories = repositories.filter(r => {
const repoTopics: string[] = r.repositoryTopics;
@@ -241,6 +244,7 @@ export class GithubEntityProvider implements EntityProvider, EventSubscriber {
!r.isArchived &&
(!repositoryFilter || repositoryFilter.test(r.name)) &&
satisfiesTopicFilter(repoTopics, topicFilters) &&
satisfiesForkFilter(allowForks, r.isFork) &&
r.defaultBranchRef
);
});
@@ -302,6 +306,7 @@ export class GithubEntityProvider implements EntityProvider, EventSubscriber {
defaultBranchRef: event.repository.default_branch,
repositoryTopics: event.repository.topics,
isArchived: event.repository.archived,
isFork: event.repository.fork,
// we can consider this file present because
// only the catalog file will be recovered from the commits
isCatalogInfoFilePresent: true,
@@ -78,6 +78,12 @@ describe('readProviderConfigs', () => {
},
},
},
providerWithForkFilter: {
organization: 'test-org6',
filters: {
allowForks: false,
},
},
providerWithHost: {
organization: 'test-org1',
host: 'ghe.internal.com',
@@ -97,7 +103,7 @@ describe('readProviderConfigs', () => {
});
const providerConfigs = readProviderConfigs(config);
expect(providerConfigs).toHaveLength(7);
expect(providerConfigs).toHaveLength(8);
expect(providerConfigs[0]).toEqual({
id: 'providerOrganizationOnly',
organization: 'test-org1',
@@ -106,6 +112,7 @@ describe('readProviderConfigs', () => {
filters: {
repository: undefined,
branch: undefined,
allowForks: true,
topic: {
include: undefined,
exclude: undefined,
@@ -122,6 +129,7 @@ describe('readProviderConfigs', () => {
filters: {
repository: undefined,
branch: undefined,
allowForks: true,
topic: {
include: undefined,
exclude: undefined,
@@ -138,6 +146,7 @@ describe('readProviderConfigs', () => {
filters: {
repository: /^repository.*filter$/, // repo
branch: undefined, // branch
allowForks: true,
topic: {
include: undefined,
exclude: undefined,
@@ -154,6 +163,7 @@ describe('readProviderConfigs', () => {
filters: {
repository: undefined,
branch: 'branch-name',
allowForks: true,
topic: {
include: undefined,
exclude: undefined,
@@ -170,6 +180,7 @@ describe('readProviderConfigs', () => {
filters: {
repository: undefined,
branch: undefined,
allowForks: true,
topic: {
include: ['backstage-include'],
exclude: ['backstage-exclude'],
@@ -179,6 +190,23 @@ describe('readProviderConfigs', () => {
validateLocationsExist: false,
});
expect(providerConfigs[5]).toEqual({
id: 'providerWithForkFilter',
organization: 'test-org6',
catalogPath: '/catalog-info.yaml',
host: 'github.com',
filters: {
repository: undefined,
branch: undefined,
allowForks: false,
topic: {
include: undefined,
exclude: undefined,
},
},
schedule: undefined,
validateLocationsExist: false,
});
expect(providerConfigs[6]).toEqual({
id: 'providerWithHost',
organization: 'test-org1',
catalogPath: '/catalog-info.yaml',
@@ -186,6 +214,7 @@ describe('readProviderConfigs', () => {
filters: {
repository: undefined,
branch: undefined,
allowForks: true,
topic: {
include: undefined,
exclude: undefined,
@@ -194,7 +223,7 @@ describe('readProviderConfigs', () => {
validateLocationsExist: false,
schedule: undefined,
});
expect(providerConfigs[6]).toEqual({
expect(providerConfigs[7]).toEqual({
id: 'providerWithSchedule',
organization: 'test-org1',
catalogPath: '/catalog-info.yaml',
@@ -202,6 +231,7 @@ describe('readProviderConfigs', () => {
filters: {
repository: undefined,
branch: undefined,
allowForks: true,
topic: {
include: undefined,
exclude: undefined,
@@ -32,6 +32,7 @@ export type GithubEntityProviderConfig = {
repository?: RegExp;
branch?: string;
topic?: GithubTopicFilters;
allowForks?: boolean;
};
validateLocationsExist: boolean;
schedule?: TaskScheduleDefinition;
@@ -72,6 +73,7 @@ function readProviderConfig(
const host = config.getOptionalString('host') ?? 'github.com';
const repositoryPattern = config.getOptionalString('filters.repository');
const branchPattern = config.getOptionalString('filters.branch');
const allowForks = config.getOptionalBoolean('filters.allowForks') ?? true;
const topicFilterInclude = config?.getOptionalStringArray(
'filters.topic.include',
);
@@ -103,6 +105,7 @@ function readProviderConfig(
? compileRegExp(repositoryPattern)
: undefined,
branch: branchPattern || undefined,
allowForks: allowForks,
topic: {
include: topicFilterInclude,
exclude: topicFilterExclude,