From 41790a7168af18dc3008f62db755d26038eb2c1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Thu, 23 Sep 2021 09:36:12 +0200 Subject: [PATCH] split up the enforcer into type + class and fix the api exports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/dirty-spoons-scream.md | 6 ++ plugins/catalog-backend/api-report.md | 25 +++++++- .../src/ingestion/CatalogRules.test.ts | 34 ++++++----- .../src/ingestion/CatalogRules.ts | 58 +++++++++++-------- .../catalog-backend/src/ingestion/index.ts | 2 + .../src/next/NextCatalogBuilder.ts | 4 +- ...faultCatalogProcessingOrchestrator.test.ts | 2 +- .../src/service/CatalogBuilder.ts | 4 +- 8 files changed, 88 insertions(+), 47 deletions(-) diff --git a/.changeset/dirty-spoons-scream.md b/.changeset/dirty-spoons-scream.md index 8d8d50a6eb..2855862d75 100644 --- a/.changeset/dirty-spoons-scream.md +++ b/.changeset/dirty-spoons-scream.md @@ -2,6 +2,8 @@ '@backstage/plugin-catalog-backend': minor --- +#### Enforcing catalog rules + Apply the catalog rules enforcer, based on origin location. This is a breaking change, in the sense that this was not properly checked in earlier versions of the new catalog engine. You may see ingestion of certain entities start to be rejected after this update, if the following conditions apply to you: @@ -41,3 +43,7 @@ catalog: If you are not using any of those `rules` section, you should not be affected by this change. If you do use any of those `rules` sections, make sure that they are complete and list all of the kinds that are in active use in your Backstage installation. + +#### Other + +Also, the class `CatalogRulesEnforcer` was renamed to `DefaultCatalogRulesEnforcer`, implementing the type `CatalogRulesEnforcer`. diff --git a/plugins/catalog-backend/api-report.md b/plugins/catalog-backend/api-report.md index c8c77c7514..2422221d6d 100644 --- a/plugins/catalog-backend/api-report.md +++ b/plugins/catalog-backend/api-report.md @@ -385,6 +385,22 @@ export type CatalogProcessorResult = | CatalogProcessorRelationResult | CatalogProcessorErrorResult; +// @public +export type CatalogRule = { + allow: Array<{ + kind: string; + }>; + locations?: Array<{ + target?: string; + type: string; + }>; +}; + +// @public +export type CatalogRulesEnforcer = { + isAllowed(entity: Entity, location: LocationSpec): boolean; +}; + // Warning: (ae-missing-release-tag) "CodeOwnersProcessor" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // // @public (undocumented) @@ -757,6 +773,14 @@ export class DefaultCatalogProcessingOrchestrator process(request: EntityProcessingRequest): Promise; } +// @public +export class DefaultCatalogRulesEnforcer implements CatalogRulesEnforcer { + constructor(rules: CatalogRule[]); + static readonly defaultRules: CatalogRule[]; + static fromConfig(config: Config): DefaultCatalogRulesEnforcer; + isAllowed(entity: Entity, location: LocationSpec): boolean; +} + // Warning: (ae-missing-release-tag) "DeferredEntity" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // // @public (undocumented) @@ -1498,5 +1522,4 @@ export class UrlReaderProcessor implements CatalogProcessor { // src/ingestion/processors/types.d.ts:58:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen // src/ingestion/types.d.ts:17:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen // src/ingestion/types.d.ts:41:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen -// src/next/processing/DefaultCatalogProcessingOrchestrator.d.ts:15:9 - (ae-forgotten-export) The symbol "CatalogRulesEnforcer" needs to be exported by the entry point index.d.ts ``` diff --git a/plugins/catalog-backend/src/ingestion/CatalogRules.test.ts b/plugins/catalog-backend/src/ingestion/CatalogRules.test.ts index 82a548a7f1..24d6b2fd66 100644 --- a/plugins/catalog-backend/src/ingestion/CatalogRules.test.ts +++ b/plugins/catalog-backend/src/ingestion/CatalogRules.test.ts @@ -14,9 +14,9 @@ * limitations under the License. */ -import { LocationSpec, Entity } from '@backstage/catalog-model'; -import { CatalogRulesEnforcer } from './CatalogRules'; +import { Entity, LocationSpec } from '@backstage/catalog-model'; import { ConfigReader } from '@backstage/config'; +import { DefaultCatalogRulesEnforcer } from './CatalogRules'; const entity = { user: { @@ -48,9 +48,9 @@ const location: Record = { }, }; -describe('CatalogRulesEnforcer', () => { +describe('DefaultCatalogRulesEnforcer', () => { it('should deny by default', () => { - const enforcer = new CatalogRulesEnforcer([]); + const enforcer = new DefaultCatalogRulesEnforcer([]); expect(enforcer.isAllowed(entity.user, location.x)).toBe(false); expect(enforcer.isAllowed(entity.group, location.y)).toBe(false); expect(enforcer.isAllowed(entity.component, location.z)).toBe(false); @@ -58,7 +58,7 @@ describe('CatalogRulesEnforcer', () => { }); it('should deny all', () => { - const enforcer = new CatalogRulesEnforcer([{ allow: [] }]); + const enforcer = new DefaultCatalogRulesEnforcer([{ allow: [] }]); expect(enforcer.isAllowed(entity.user, location.x)).toBe(false); expect(enforcer.isAllowed(entity.group, location.y)).toBe(false); expect(enforcer.isAllowed(entity.component, location.z)).toBe(false); @@ -66,7 +66,7 @@ describe('CatalogRulesEnforcer', () => { }); it('should allow all', () => { - const enforcer = new CatalogRulesEnforcer([ + const enforcer = new DefaultCatalogRulesEnforcer([ { allow: ['User', 'Group', 'Component', 'Location'].map(kind => ({ kind, @@ -80,7 +80,7 @@ describe('CatalogRulesEnforcer', () => { }); it('should deny groups', () => { - const enforcer = new CatalogRulesEnforcer([ + const enforcer = new DefaultCatalogRulesEnforcer([ { allow: [{ kind: 'User' }, { kind: 'Component' }] }, ]); expect(enforcer.isAllowed(entity.user, location.x)).toBe(true); @@ -91,7 +91,7 @@ describe('CatalogRulesEnforcer', () => { }); it('should deny groups from github', () => { - const enforcer = new CatalogRulesEnforcer([ + const enforcer = new DefaultCatalogRulesEnforcer([ { allow: [{ kind: 'User' }, { kind: 'Component' }] }, { allow: [{ kind: 'Group' }], locations: [{ type: 'file' }] }, ]); @@ -103,7 +103,7 @@ describe('CatalogRulesEnforcer', () => { }); it('should allow groups from files', () => { - const enforcer = new CatalogRulesEnforcer([ + const enforcer = new DefaultCatalogRulesEnforcer([ { allow: [{ kind: 'Group' }], locations: [{ type: 'file' }] }, ]); expect(enforcer.isAllowed(entity.user, location.x)).toBe(false); @@ -114,7 +114,7 @@ describe('CatalogRulesEnforcer', () => { }); it('should not be sensitive to kind case', () => { - const enforcer = new CatalogRulesEnforcer([ + const enforcer = new DefaultCatalogRulesEnforcer([ { allow: [{ kind: 'group' }] }, { allow: [{ kind: 'Component' }] }, ]); @@ -127,7 +127,9 @@ describe('CatalogRulesEnforcer', () => { describe('fromConfig', () => { it('should allow components by default', () => { - const enforcer = CatalogRulesEnforcer.fromConfig(new ConfigReader({})); + const enforcer = DefaultCatalogRulesEnforcer.fromConfig( + new ConfigReader({}), + ); expect(enforcer.isAllowed(entity.user, location.x)).toBe(false); expect(enforcer.isAllowed(entity.group, location.y)).toBe(false); expect(enforcer.isAllowed(entity.component, location.z)).toBe(true); @@ -135,7 +137,7 @@ describe('CatalogRulesEnforcer', () => { }); it('should deny all', () => { - const enforcer = CatalogRulesEnforcer.fromConfig( + const enforcer = DefaultCatalogRulesEnforcer.fromConfig( new ConfigReader({ catalog: { rules: [] } }), ); expect(enforcer.isAllowed(entity.user, location.x)).toBe(false); @@ -145,7 +147,7 @@ describe('CatalogRulesEnforcer', () => { }); it('should allow all', () => { - const enforcer = CatalogRulesEnforcer.fromConfig( + const enforcer = DefaultCatalogRulesEnforcer.fromConfig( new ConfigReader({ catalog: { rules: [{ allow: ['User', 'Group'] }, { allow: ['Component'] }], @@ -158,7 +160,7 @@ describe('CatalogRulesEnforcer', () => { }); it('should deny groups', () => { - const enforcer = CatalogRulesEnforcer.fromConfig( + const enforcer = DefaultCatalogRulesEnforcer.fromConfig( new ConfigReader({ catalog: { rules: [{ allow: ['User'] }, { allow: ['Component'] }] }, }), @@ -172,7 +174,7 @@ describe('CatalogRulesEnforcer', () => { }); it('should allow groups from a specific github location', () => { - const enforcer = CatalogRulesEnforcer.fromConfig( + const enforcer = DefaultCatalogRulesEnforcer.fromConfig( new ConfigReader({ catalog: { rules: [{ allow: ['user'] }], @@ -199,7 +201,7 @@ describe('CatalogRulesEnforcer', () => { }); it('should not care about location configuration in catalog.rules', () => { - const enforcer = CatalogRulesEnforcer.fromConfig( + const enforcer = DefaultCatalogRulesEnforcer.fromConfig( new ConfigReader({ catalog: { rules: [{ allow: ['Group'], locations: [{ type: 'github' }] }], diff --git a/plugins/catalog-backend/src/ingestion/CatalogRules.ts b/plugins/catalog-backend/src/ingestion/CatalogRules.ts index 4902a3655d..a0ef5f3d53 100644 --- a/plugins/catalog-backend/src/ingestion/CatalogRules.ts +++ b/plugins/catalog-backend/src/ingestion/CatalogRules.ts @@ -19,31 +19,39 @@ import { LocationSpec, Entity } from '@backstage/catalog-model'; import path from 'path'; /** - * A structure for matching entities to a given rule. - */ -type EntityMatcher = { - kind: string; -}; - -/** - * A structure for matching locations to a given rule. - */ -type LocationMatcher = { - target?: string; - type: string; -}; - -/** - * Rules to apply to catalog entities + * Rules to apply to catalog entities. * - * An undefined list of matchers means match all, an empty list of matchers means match none + * An undefined list of matchers means match all, an empty list of matchers means match none. + * + * @public */ -type CatalogRule = { - allow: EntityMatcher[]; - locations?: LocationMatcher[]; +export type CatalogRule = { + allow: Array<{ + kind: string; + }>; + locations?: Array<{ + target?: string; + type: string; + }>; }; -export class CatalogRulesEnforcer { +/** + * Decides whether an entity from a given location is allowed to enter the + * catalog, according to some rule set. + * + * @public + */ +export type CatalogRulesEnforcer = { + isAllowed(entity: Entity, location: LocationSpec): boolean; +}; + +/** + * Implements the default catalog rule set, consuming the config keys + * `catalog.rules` and `catalog.locations.[].rules`. + * + * @public + */ +export class DefaultCatalogRulesEnforcer implements CatalogRulesEnforcer { /** * Default rules used by the catalog. * @@ -94,7 +102,7 @@ export class CatalogRulesEnforcer { })); rules.push(...globalRules); } else { - rules.push(...CatalogRulesEnforcer.defaultRules); + rules.push(...DefaultCatalogRulesEnforcer.defaultRules); } if (config.has('catalog.locations')) { @@ -116,7 +124,7 @@ export class CatalogRulesEnforcer { rules.push(...locationRules); } - return new CatalogRulesEnforcer(rules); + return new DefaultCatalogRulesEnforcer(rules); } constructor(private readonly rules: CatalogRule[]) {} @@ -141,7 +149,7 @@ export class CatalogRulesEnforcer { private matchLocation( location: LocationSpec, - matchers?: LocationMatcher[], + matchers?: { target?: string; type: string }[], ): boolean { if (!matchers) { return true; @@ -160,7 +168,7 @@ export class CatalogRulesEnforcer { return false; } - private matchEntity(entity: Entity, matchers?: EntityMatcher[]): boolean { + private matchEntity(entity: Entity, matchers?: { kind: string }[]): boolean { if (!matchers) { return true; } diff --git a/plugins/catalog-backend/src/ingestion/index.ts b/plugins/catalog-backend/src/ingestion/index.ts index e27a3ef0ac..80fea962b9 100644 --- a/plugins/catalog-backend/src/ingestion/index.ts +++ b/plugins/catalog-backend/src/ingestion/index.ts @@ -14,6 +14,8 @@ * limitations under the License. */ +export type { CatalogRule, CatalogRulesEnforcer } from './CatalogRules'; +export { DefaultCatalogRulesEnforcer } from './CatalogRules'; export { HigherOrderOperations } from './HigherOrderOperations'; export { LocationReaders } from './LocationReaders'; export type { diff --git a/plugins/catalog-backend/src/next/NextCatalogBuilder.ts b/plugins/catalog-backend/src/next/NextCatalogBuilder.ts index 3e281e4e38..6caab23545 100644 --- a/plugins/catalog-backend/src/next/NextCatalogBuilder.ts +++ b/plugins/catalog-backend/src/next/NextCatalogBuilder.ts @@ -78,7 +78,7 @@ import { import { CatalogEnvironment } from '../service/CatalogBuilder'; import { createNextRouter } from './NextRouter'; import { DefaultRefreshService } from './DefaultRefreshService'; -import { CatalogRulesEnforcer } from '../ingestion/CatalogRules'; +import { DefaultCatalogRulesEnforcer } from '../ingestion/CatalogRules'; /** * A builder that helps wire up all of the component parts of the catalog. @@ -305,7 +305,7 @@ export class NextCatalogBuilder { refreshInterval: this.refreshInterval, }); const integrations = ScmIntegrations.fromConfig(config); - const rulesEnforcer = CatalogRulesEnforcer.fromConfig(config); + const rulesEnforcer = DefaultCatalogRulesEnforcer.fromConfig(config); const orchestrator = new DefaultCatalogProcessingOrchestrator({ processors, integrations, diff --git a/plugins/catalog-backend/src/next/processing/DefaultCatalogProcessingOrchestrator.test.ts b/plugins/catalog-backend/src/next/processing/DefaultCatalogProcessingOrchestrator.test.ts index 1051557a0d..c93734b37f 100644 --- a/plugins/catalog-backend/src/next/processing/DefaultCatalogProcessingOrchestrator.test.ts +++ b/plugins/catalog-backend/src/next/processing/DefaultCatalogProcessingOrchestrator.test.ts @@ -62,7 +62,7 @@ describe('DefaultCatalogProcessingOrchestrator', () => { }; const rulesEnforcer: jest.Mocked = { isAllowed: jest.fn(), - } as any; + }; const orchestrator = new DefaultCatalogProcessingOrchestrator({ processors: [processor], diff --git a/plugins/catalog-backend/src/service/CatalogBuilder.ts b/plugins/catalog-backend/src/service/CatalogBuilder.ts index e32e071ebc..40683bb4bf 100644 --- a/plugins/catalog-backend/src/service/CatalogBuilder.ts +++ b/plugins/catalog-backend/src/service/CatalogBuilder.ts @@ -56,7 +56,7 @@ import { StaticLocationProcessor, UrlReaderProcessor, } from '../ingestion'; -import { CatalogRulesEnforcer } from '../ingestion/CatalogRules'; +import { DefaultCatalogRulesEnforcer } from '../ingestion/CatalogRules'; import { RepoLocationAnalyzer } from '../ingestion/LocationAnalyzer'; import { jsonPlaceholderResolver, @@ -240,7 +240,7 @@ export class CatalogBuilder { const policy = this.buildEntityPolicy(); const processors = this.buildProcessors(); - const rulesEnforcer = CatalogRulesEnforcer.fromConfig(config); + const rulesEnforcer = DefaultCatalogRulesEnforcer.fromConfig(config); const parser = this.parser || defaultEntityDataParser; const locationReader = new LocationReaders({