diff --git a/.changeset/three-poems-think.md b/.changeset/three-poems-think.md new file mode 100644 index 0000000000..aad3cae117 --- /dev/null +++ b/.changeset/three-poems-think.md @@ -0,0 +1,14 @@ +--- +'@backstage/plugin-catalog-backend-module-github': minor +--- + +Added the ability for the GitHub discovery provider to validate that catalog files exist before emitting them. + +Users can now set the `validateLocationsExist` property to `true` in their GitHub discovery configuration to opt in to this feature. +This feature only works with `catalogPath`s that do not contain wildcards. + +When `validateLocationsExist` is set to `true`, the GitHub discovery provider will retrieve the object from the +repository at the provided `catalogPath`. +If this file exists and is non-empty, then it will be emitted as a location for further processing. +If this file does not exist or is empty, then it will not be emitted. +Not emitting locations that do not exist allows for far fewer calls to the GitHub API to validate locations that do not exist. diff --git a/docs/integrations/github/discovery.md b/docs/integrations/github/discovery.md index 092a171223..4f032901a9 100644 --- a/docs/integrations/github/discovery.md +++ b/docs/integrations/github/discovery.md @@ -104,6 +104,13 @@ catalog: topic: include: ['backstage-include'] # optional array of strings exclude: ['experiments'] # optional array of strings + validateLocationsExist: + organization: 'backstage' # string + catalogPath: '/catalog-info.yaml' # string + filters: + branch: 'main' # string + repository: '.*' # Regex + validateLocationsExist: true # optional boolean enterpriseProviderId: host: ghe.example.net organization: 'backstage' # string @@ -118,7 +125,8 @@ This provider supports multiple organizations via unique provider IDs. - **`catalogPath`** _(optional)_: Default: `/catalog-info.yaml`. Path where to look for `catalog-info.yaml` files. - You can use wildcards - `*` or `**` - to search the path and/or the filename + You can use wildcards - `*` or `**` - to search the path and/or the filename. + Wildcards cannot be used if the `validateLocationsExist` option is set to `true`. - **`filters`** _(optional)_: - **`branch`** _(optional)_: String used to filter results based on the branch name. @@ -139,6 +147,12 @@ This provider supports multiple organizations via unique provider IDs. - **`organization`**: Name of your organization account/workspace. If you want to add multiple organizations, you need to add one provider config each. +- **`validateLocationsExist`** _(optional)_: + Whether to validate locations that exist before emitting them. + This option avoids generating locations for catalog info files that do not exist in the source repository. + Defaults to `false`. + Due to limitations in the GitHub API's ability to query for repository objects, this option cannot be used in + conjunction with wildcards in the `catalogPath`. - **`schedule`** _(optional)_: - **`frequency`**: How often you want the task to run. The system does its best to avoid overlapping invocations. diff --git a/plugins/catalog-backend-module-github/src/lib/github.test.ts b/plugins/catalog-backend-module-github/src/lib/github.test.ts index e2303c16e6..25878c2005 100644 --- a/plugins/catalog-backend-module-github/src/lib/github.test.ts +++ b/plugins/catalog-backend-module-github/src/lib/github.test.ts @@ -213,6 +213,7 @@ describe('github', () => { defaultBranchRef: { name: 'main', }, + catalogInfoFile: null, }, { name: 'demo', @@ -222,6 +223,11 @@ describe('github', () => { defaultBranchRef: { name: 'main', }, + catalogInfoFile: { + __typename: 'Blob', + id: 'acb123', + text: 'some yaml', + }, }, ], pageInfo: { @@ -243,6 +249,7 @@ describe('github', () => { defaultBranchRef: { name: 'main', }, + catalogInfoFile: null, }, { name: 'demo', @@ -252,6 +259,11 @@ describe('github', () => { defaultBranchRef: { name: 'main', }, + catalogInfoFile: { + __typename: 'Blob', + id: 'acb123', + text: 'some yaml', + }, }, ], }; @@ -262,9 +274,9 @@ describe('github', () => { ), ); - await expect(getOrganizationRepositories(graphql, 'a')).resolves.toEqual( - output, - ); + await expect( + getOrganizationRepositories(graphql, 'a', 'catalog-info.yaml'), + ).resolves.toEqual(output); }); }); }); diff --git a/plugins/catalog-backend-module-github/src/lib/github.ts b/plugins/catalog-backend-module-github/src/lib/github.ts index 6a944af330..ef6a7f817a 100644 --- a/plugins/catalog-backend-module-github/src/lib/github.ts +++ b/plugins/catalog-backend-module-github/src/lib/github.ts @@ -65,6 +65,11 @@ export type Repository = { defaultBranchRef: { name: string; } | null; + catalogInfoFile: { + __typename: string; + id: string; + text: string; + } | null; }; type RepositoryTopics = { @@ -266,14 +271,30 @@ export async function getOrganizationTeams( export async function getOrganizationRepositories( client: typeof graphql, org: string, + catalogPath: string, ): Promise<{ repositories: Repository[] }> { + let relativeCatalogPathRef: string; + // We must strip the leading slash or the query for objects does not work + if (catalogPath.startsWith('/')) { + relativeCatalogPathRef = catalogPath.substring(1); + } else { + relativeCatalogPathRef = catalogPath; + } + const catalogPathRef = `HEAD:${relativeCatalogPathRef}`; const query = ` - query repositories($org: String!, $cursor: String) { + query repositories($org: String!, $catalogPathRef: String!, $cursor: String) { repositoryOwner(login: $org) { login repositories(first: 100, after: $cursor) { nodes { name + catalogInfoFile: object(expression: $catalogPathRef) { + __typename + ... on Blob { + id + text + } + } url isArchived repositoryTopics(first: 100) { @@ -302,7 +323,7 @@ export async function getOrganizationRepositories( query, r => r.repositoryOwner?.repositories, x => x, - { org }, + { org, catalogPathRef }, ); return { repositories }; diff --git a/plugins/catalog-backend-module-github/src/processors/GithubDiscoveryProcessor.test.ts b/plugins/catalog-backend-module-github/src/processors/GithubDiscoveryProcessor.test.ts index c915e8b136..b69728cc62 100644 --- a/plugins/catalog-backend-module-github/src/processors/GithubDiscoveryProcessor.test.ts +++ b/plugins/catalog-backend-module-github/src/processors/GithubDiscoveryProcessor.test.ts @@ -153,6 +153,7 @@ describe('GithubDiscoveryProcessor', () => { defaultBranchRef: { name: 'master', }, + catalogInfoFile: null, }, { name: 'demo', @@ -162,6 +163,7 @@ describe('GithubDiscoveryProcessor', () => { defaultBranchRef: { name: 'main', }, + catalogInfoFile: null, }, ], }); @@ -203,6 +205,7 @@ describe('GithubDiscoveryProcessor', () => { defaultBranchRef: { name: 'main', }, + catalogInfoFile: null, }, ], }); @@ -234,6 +237,7 @@ describe('GithubDiscoveryProcessor', () => { repositoryTopics: { nodes: [] }, isArchived: false, defaultBranchRef: null, + catalogInfoFile: null, }, ], }); @@ -259,6 +263,7 @@ describe('GithubDiscoveryProcessor', () => { defaultBranchRef: { name: 'master', }, + catalogInfoFile: null, }, ], }); @@ -293,6 +298,7 @@ describe('GithubDiscoveryProcessor', () => { defaultBranchRef: { name: 'main', }, + catalogInfoFile: null, }, { name: 'techdocs-cli', @@ -302,6 +308,7 @@ describe('GithubDiscoveryProcessor', () => { defaultBranchRef: { name: 'main', }, + catalogInfoFile: null, }, { name: 'techdocs-container', @@ -311,6 +318,7 @@ describe('GithubDiscoveryProcessor', () => { defaultBranchRef: { name: 'main', }, + catalogInfoFile: null, }, { name: 'techdocs-durp', @@ -318,6 +326,7 @@ describe('GithubDiscoveryProcessor', () => { repositoryTopics: { nodes: [] }, isArchived: false, defaultBranchRef: null, + catalogInfoFile: null, }, ], }); @@ -360,6 +369,7 @@ describe('GithubDiscoveryProcessor', () => { defaultBranchRef: { name: 'main', }, + catalogInfoFile: null, }, { name: 'test', @@ -369,6 +379,7 @@ describe('GithubDiscoveryProcessor', () => { defaultBranchRef: { name: 'main', }, + catalogInfoFile: null, }, { name: 'test-archived', @@ -378,6 +389,7 @@ describe('GithubDiscoveryProcessor', () => { defaultBranchRef: { name: 'main', }, + catalogInfoFile: null, }, { name: 'testxyz', @@ -387,6 +399,7 @@ describe('GithubDiscoveryProcessor', () => { defaultBranchRef: { name: 'main', }, + catalogInfoFile: null, }, ], }); diff --git a/plugins/catalog-backend-module-github/src/processors/GithubDiscoveryProcessor.ts b/plugins/catalog-backend-module-github/src/processors/GithubDiscoveryProcessor.ts index 97c77bf033..593f5f72f1 100644 --- a/plugins/catalog-backend-module-github/src/processors/GithubDiscoveryProcessor.ts +++ b/plugins/catalog-backend-module-github/src/processors/GithubDiscoveryProcessor.ts @@ -121,7 +121,11 @@ export class GithubDiscoveryProcessor implements CatalogProcessor { const startTimestamp = Date.now(); this.logger.info(`Reading GitHub repositories from ${location.target}`); - const { repositories } = await getOrganizationRepositories(client, org); + const { repositories } = await getOrganizationRepositories( + client, + org, + catalogPath, + ); const matching = repositories.filter( r => !r.isArchived && repoSearchPath.test(r.name), ); diff --git a/plugins/catalog-backend-module-github/src/providers/GithubEntityProvider.test.ts b/plugins/catalog-backend-module-github/src/providers/GithubEntityProvider.test.ts index a63069d552..cc9a9bfee3 100644 --- a/plugins/catalog-backend-module-github/src/providers/GithubEntityProvider.test.ts +++ b/plugins/catalog-backend-module-github/src/providers/GithubEntityProvider.test.ts @@ -174,6 +174,11 @@ describe('GithubEntityProvider', () => { defaultBranchRef: { name: 'main', }, + catalogInfoFile: { + __typename: 'Blob', + id: 'abc123', + text: 'some yaml', + }, }, ], }), @@ -271,6 +276,11 @@ describe('GithubEntityProvider', () => { defaultBranchRef: { name: 'main', }, + catalogInfoFile: { + __typename: 'Blob', + id: 'abc123', + text: 'some yaml', + }, }, ], }), @@ -345,6 +355,11 @@ describe('GithubEntityProvider', () => { defaultBranchRef: { name: 'main', }, + catalogInfoFile: { + __typename: 'Blob', + id: 'abc123', + text: 'some yaml', + }, }, ], }), @@ -386,6 +401,110 @@ describe('GithubEntityProvider', () => { }); }); + it('should filter out invalid locations when validateLocationsExist is set to true', async () => { + const config = new ConfigReader({ + catalog: { + providers: { + github: { + myProvider: { + organization: 'test-org', + catalogPath: 'catalog-custom.yaml', + filters: { + branch: 'main', + }, + validateLocationsExist: true, + }, + }, + }, + }, + }); + const schedule = new PersistingTaskRunner(); + const entityProviderConnection: EntityProviderConnection = { + applyMutation: jest.fn(), + refresh: jest.fn(), + }; + + const provider = GithubEntityProvider.fromConfig(config, { + logger, + schedule, + })[0]; + + const mockGetOrganizationRepositories = jest.spyOn( + helpers, + 'getOrganizationRepositories', + ); + + mockGetOrganizationRepositories.mockReturnValue( + Promise.resolve({ + repositories: [ + { + name: 'test-repo', + url: 'https://github.com/test-org/test-repo', + repositoryTopics: { + nodes: [], + }, + isArchived: false, + defaultBranchRef: { + name: 'main', + }, + catalogInfoFile: null, + }, + { + name: 'another-repo', + url: 'https://github.com/test-org/another-repo', + repositoryTopics: { + nodes: [], + }, + isArchived: false, + defaultBranchRef: { + name: 'main', + }, + catalogInfoFile: { + __typename: 'Blob', + id: 'abc123', + text: 'some yaml', + }, + }, + ], + }), + ); + + await provider.connect(entityProviderConnection); + + const taskDef = schedule.getTasks()[0]; + expect(taskDef.id).toEqual('github-provider:myProvider:refresh'); + await (taskDef.fn as () => Promise)(); + + const url = `https://github.com/test-org/another-repo/blob/main/catalog-custom.yaml`; + const expectedEntities = [ + { + entity: { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Location', + metadata: { + annotations: { + 'backstage.io/managed-by-location': `url:${url}`, + 'backstage.io/managed-by-origin-location': `url:${url}`, + }, + name: 'generated-934f500db2ba2e8ea3524567926f45a73bb0b532', + }, + spec: { + presence: 'optional', + target: `${url}`, + type: 'url', + }, + }, + locationKey: 'github-provider:myProvider', + }, + ]; + + expect(entityProviderConnection.applyMutation).toHaveBeenCalledTimes(1); + expect(entityProviderConnection.applyMutation).toHaveBeenCalledWith({ + type: 'full', + entities: expectedEntities, + }); + }); + it('apply full update on scheduled execution with topic exclusion taking priority over topic inclusion', async () => { const config = new ConfigReader({ catalog: { @@ -440,6 +559,11 @@ describe('GithubEntityProvider', () => { defaultBranchRef: { name: 'main', }, + catalogInfoFile: { + __typename: 'Blob', + id: 'abc123', + text: 'some yaml', + }, }, { name: 'test-repo-2', @@ -458,6 +582,11 @@ describe('GithubEntityProvider', () => { defaultBranchRef: { name: 'main', }, + catalogInfoFile: { + __typename: 'Blob', + id: 'abc123', + text: 'some yaml', + }, }, { name: 'test-repo-3', @@ -473,6 +602,11 @@ describe('GithubEntityProvider', () => { defaultBranchRef: { name: 'main', }, + catalogInfoFile: { + __typename: 'Blob', + id: 'abc123', + text: 'some yaml', + }, }, ], }), diff --git a/plugins/catalog-backend-module-github/src/providers/GithubEntityProvider.ts b/plugins/catalog-backend-module-github/src/providers/GithubEntityProvider.ts index 378e1a96a7..6d3badf865 100644 --- a/plugins/catalog-backend-module-github/src/providers/GithubEntityProvider.ts +++ b/plugins/catalog-backend-module-github/src/providers/GithubEntityProvider.ts @@ -178,6 +178,7 @@ export class GithubEntityProvider implements EntityProvider { private async findCatalogFiles(): Promise { const organization = this.config.organization; const host = this.integration.host; + const catalogPath = this.config.catalogPath; const orgUrl = `https://${host}/${organization}`; const { headers } = await this.githubCredentialsProvider.getCredentials({ @@ -192,8 +193,18 @@ export class GithubEntityProvider implements EntityProvider { const { repositories } = await getOrganizationRepositories( client, organization, + catalogPath, ); + if (this.config.validateLocationsExist) { + return repositories.filter(repository => { + return ( + repository.catalogInfoFile?.__typename === 'Blob' && + repository.catalogInfoFile.text !== '' + ); + }); + } + return repositories; } diff --git a/plugins/catalog-backend-module-github/src/providers/GithubEntityProviderConfig.test.ts b/plugins/catalog-backend-module-github/src/providers/GithubEntityProviderConfig.test.ts index c80546bdff..0d9b5ce65b 100644 --- a/plugins/catalog-backend-module-github/src/providers/GithubEntityProviderConfig.test.ts +++ b/plugins/catalog-backend-module-github/src/providers/GithubEntityProviderConfig.test.ts @@ -112,6 +112,7 @@ describe('readProviderConfigs', () => { }, }, schedule: undefined, + validateLocationsExist: false, }); expect(providerConfigs[1]).toEqual({ id: 'providerCustomCatalogPath', @@ -127,6 +128,7 @@ describe('readProviderConfigs', () => { }, }, schedule: undefined, + validateLocationsExist: false, }); expect(providerConfigs[2]).toEqual({ id: 'providerWithRepositoryFilter', @@ -142,6 +144,7 @@ describe('readProviderConfigs', () => { }, }, schedule: undefined, + validateLocationsExist: false, }); expect(providerConfigs[3]).toEqual({ id: 'providerWithBranchFilter', @@ -157,6 +160,7 @@ describe('readProviderConfigs', () => { }, }, schedule: undefined, + validateLocationsExist: false, }); expect(providerConfigs[4]).toEqual({ id: 'providerWithTopicFilter', @@ -172,6 +176,7 @@ describe('readProviderConfigs', () => { }, }, schedule: undefined, + validateLocationsExist: false, }); expect(providerConfigs[5]).toEqual({ id: 'providerWithHost', @@ -186,6 +191,7 @@ describe('readProviderConfigs', () => { exclude: undefined, }, }, + validateLocationsExist: false, schedule: undefined, }); expect(providerConfigs[6]).toEqual({ @@ -207,6 +213,38 @@ describe('readProviderConfigs', () => { minutes: 3, }, }, + validateLocationsExist: false, }); }); + + it('defaults validateLocationsExist to false', () => { + const config = new ConfigReader({ + catalog: { + providers: { + github: { + organization: 'test-org', + }, + }, + }, + }); + const providerConfigs = readProviderConfigs(config); + + expect(providerConfigs[0].validateLocationsExist).toEqual(false); + }); + + it('throws an error when a wildcard catalog path is configured with validation of locations', () => { + const config = new ConfigReader({ + catalog: { + providers: { + github: { + organization: 'test-org', + validateLocationsExist: true, + catalogPath: '/*/catalog-info.yaml', + }, + }, + }, + }); + + expect(() => readProviderConfigs(config)).toThrow(); + }); }); diff --git a/plugins/catalog-backend-module-github/src/providers/GithubEntityProviderConfig.ts b/plugins/catalog-backend-module-github/src/providers/GithubEntityProviderConfig.ts index 18c0a3915d..e888743258 100644 --- a/plugins/catalog-backend-module-github/src/providers/GithubEntityProviderConfig.ts +++ b/plugins/catalog-backend-module-github/src/providers/GithubEntityProviderConfig.ts @@ -33,6 +33,7 @@ export type GithubEntityProviderConfig = { branch?: string; topic?: GithubTopicFilters; }; + validateLocationsExist: boolean; schedule?: TaskScheduleDefinition; }; @@ -77,6 +78,16 @@ function readProviderConfig( const topicFilterExclude = config?.getOptionalStringArray( 'filters.topic.exclude', ); + const validateLocationsExist = + config?.getOptionalBoolean('validateLocationsExist') ?? false; + + const catalogPathContainsWildcard = catalogPath.includes('*'); + + if (validateLocationsExist && catalogPathContainsWildcard) { + throw Error( + `Error while processing GitHub provider config. The catalog path ${catalogPath} contains a wildcard, which is incompatible with validation of locations existing before emitting them. Ensure that validateLocationsExist is set to false.`, + ); + } const schedule = config.has('schedule') ? readTaskScheduleDefinitionFromConfig(config.getConfig('schedule')) @@ -98,6 +109,7 @@ function readProviderConfig( }, }, schedule, + validateLocationsExist, }; }