paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 47 Packages Projects Releases Wiki Activity

Merge pull request #14058 from Niksko/github-discovery-validate

Browse Source 
Optionally validate GitHub discovery locations before emitting
This commit is contained in:
Fredrik Adelöw
2022-10-20 13:46:21 +01:00
committed by GitHub
parent dd308381a0 e2d088426b
commit d57627f917
10 changed files with 280 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/three-poems-think.md
+14
View File
@@ -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.
docs/integrations/github/discovery.md
+15 -1
View File
@@ -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.
plugins/catalog-backend-module-github/src/lib/github.test.ts
+15 -3
View File
@@ -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);
});
});
});
plugins/catalog-backend-module-github/src/lib/github.ts
+23 -2
View File
@@ -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 };
plugins/catalog-backend-module-github/src/processors/GithubDiscoveryProcessor.test.ts
+13
View File
@@ -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,
},
],
});
plugins/catalog-backend-module-github/src/processors/GithubDiscoveryProcessor.ts
+5 -1
View File
@@ -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),
);
plugins/catalog-backend-module-github/src/providers/GithubEntityProvider.test.ts
+134
View File
@@ -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<void>)();
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',
},
},
],
}),
plugins/catalog-backend-module-github/src/providers/GithubEntityProvider.ts
+11
View File
@@ -178,6 +178,7 @@ export class GithubEntityProvider implements EntityProvider {
private async findCatalogFiles(): Promise<Repository[]> {
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;
}
plugins/catalog-backend-module-github/src/providers/GithubEntityProviderConfig.test.ts
+38
View File
@@ -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();
});
});
plugins/catalog-backend-module-github/src/providers/GithubEntityProviderConfig.ts
+12
View File
@@ -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,
};
}