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

Merge pull request #7270 from backstage/freben/enforce-better

Browse Source 
Apply the rules enforcer, based on origin location
This commit is contained in:
Fredrik Adelöw
2021-09-23 11:51:57 +02:00
committed by GitHub
parent a1935038e8 41790a7168
commit ccaafec13b
9 changed files with 255 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/dirty-spoons-scream.md
+49
View File
@@ -0,0 +1,49 @@
---
'@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:
- You are using the configuration key `catalog.rules.[].allow`, and
- Your registered locations point (directly or transitively) to entities whose kinds are not listed in `catalog.rules.[].allow`
and/or
- You are using the configuration key `catalog.locations.[].rules.[].allow`
- The config locations point (directly or transitively) to entities whose kinds are not listed neither `catalog.rules.[].allow`, nor in the corresponding `.rules.[].allow` of that config location
This is an example of what the configuration might look like:
```yaml
catalog:
# These do not list Template as a valid kind; users are therefore unable to
# manually register entities of the Template kind
rules:
- allow:
- Component
- API
- Resource
- Group
- User
- System
- Domain
- Location
locations:
# This lists Template as valid only for that specific config location
- type: file
target: ../../plugins/scaffolder-backend/sample-templates/all-templates.yaml
rules:
- allow: [Template]
```
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`.
plugins/catalog-backend/api-report.md
+25
View File
@@ -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)
@@ -751,11 +767,20 @@ export class DefaultCatalogProcessingOrchestrator
logger: Logger_2;
parser: CatalogProcessorParser;
policy: EntityPolicy;
rulesEnforcer: CatalogRulesEnforcer;
});
// (undocumented)
process(request: EntityProcessingRequest): Promise<EntityProcessingResult>;
}
// @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)
plugins/catalog-backend/src/ingestion/CatalogRules.test.ts
+18 -16
View File
@@ -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<string, LocationSpec> = {
},
};
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' }] }],
plugins/catalog-backend/src/ingestion/CatalogRules.ts
+43 -26
View File
@@ -16,33 +16,42 @@
import { Config } from '@backstage/config';
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.
*
@@ -93,7 +102,7 @@ export class CatalogRulesEnforcer {
}));
rules.push(...globalRules);
} else {
rules.push(...CatalogRulesEnforcer.defaultRules);
rules.push(...DefaultCatalogRulesEnforcer.defaultRules);
}
if (config.has('catalog.locations')) {
@@ -104,7 +113,7 @@ export class CatalogRulesEnforcer {
return [];
}
const type = locConf.getString('type');
const target = locConf.getString('target');
const target = resolveTarget(type, locConf.getString('target'));
return locConf.getConfigArray('rules').map(ruleConf => ({
allow: ruleConf.getStringArray('allow').map(kind => ({ kind })),
@@ -115,7 +124,7 @@ export class CatalogRulesEnforcer {
rules.push(...locationRules);
}
return new CatalogRulesEnforcer(rules);
return new DefaultCatalogRulesEnforcer(rules);
}
constructor(private readonly rules: CatalogRule[]) {}
@@ -140,7 +149,7 @@ export class CatalogRulesEnforcer {
private matchLocation(
location: LocationSpec,
matchers?: LocationMatcher[],
matchers?: { target?: string; type: string }[],
): boolean {
if (!matchers) {
return true;
@@ -159,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;
}
@@ -175,3 +184,11 @@ export class CatalogRulesEnforcer {
return false;
}
}
function resolveTarget(type: string, target: string): string {
if (type !== 'file') {
return target;
}
return path.resolve(target);
}
plugins/catalog-backend/src/ingestion/index.ts
+2
View File
@@ -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 {
plugins/catalog-backend/src/next/NextCatalogBuilder.ts
+3
View File
@@ -78,6 +78,7 @@ import {
import { CatalogEnvironment } from '../service/CatalogBuilder';
import { createNextRouter } from './NextRouter';
import { DefaultRefreshService } from './DefaultRefreshService';
import { DefaultCatalogRulesEnforcer } from '../ingestion/CatalogRules';
/**
* A builder that helps wire up all of the component parts of the catalog.
@@ -304,9 +305,11 @@ export class NextCatalogBuilder {
refreshInterval: this.refreshInterval,
});
const integrations = ScmIntegrations.fromConfig(config);
const rulesEnforcer = DefaultCatalogRulesEnforcer.fromConfig(config);
const orchestrator = new DefaultCatalogProcessingOrchestrator({
processors,
integrations,
rulesEnforcer,
logger,
parser,
policy,
plugins/catalog-backend/src/next/processing/DefaultCatalogProcessingOrchestrator.test.ts
+86
View File
@@ -0,0 +1,86 @@
/*
* Copyright 2021 The Backstage Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { getVoidLogger } from '@backstage/backend-common';
import {
EntityPolicy,
LocationEntity,
LOCATION_ANNOTATION,
ORIGIN_LOCATION_ANNOTATION,
} from '@backstage/catalog-model';
import { ScmIntegrationRegistry } from '@backstage/integration';
import {
CatalogProcessor,
CatalogProcessorParser,
results,
} from '../../ingestion';
import { CatalogRulesEnforcer } from '../../ingestion/CatalogRules';
import { DefaultCatalogProcessingOrchestrator } from './DefaultCatalogProcessingOrchestrator';
describe('DefaultCatalogProcessingOrchestrator', () => {
it('enforces catalog rules', async () => {
const entity: LocationEntity = {
apiVersion: 'backstage.io/v1beta1',
kind: 'Location',
metadata: {
name: 'l',
annotations: {
[ORIGIN_LOCATION_ANNOTATION]: 'url:https://example.com/origin.yaml',
[LOCATION_ANNOTATION]: 'url:https://example.com/origin.yaml',
},
},
spec: {
type: 'url',
target: 'http://example.com/entity.yaml',
},
};
const processor: jest.Mocked<CatalogProcessor> = {
validateEntityKind: jest.fn(async () => true),
readLocation: jest.fn(async (_l, _o, emit) => {
emit(results.entity({ type: 't', target: 't' }, entity));
return true;
}),
};
const integrations: jest.Mocked<ScmIntegrationRegistry> = {} as any;
const parser: CatalogProcessorParser = jest.fn();
const policy: jest.Mocked<EntityPolicy> = {
enforce: jest.fn(async x => x),
};
const rulesEnforcer: jest.Mocked<CatalogRulesEnforcer> = {
isAllowed: jest.fn(),
};
const orchestrator = new DefaultCatalogProcessingOrchestrator({
processors: [processor],
integrations,
logger: getVoidLogger(),
parser,
policy,
rulesEnforcer,
});
rulesEnforcer.isAllowed.mockReturnValueOnce(true);
await expect(
orchestrator.process({ entity, state: new Map() }),
).resolves.toEqual(expect.objectContaining({ ok: true }));
rulesEnforcer.isAllowed.mockReturnValueOnce(false);
await expect(
orchestrator.process({ entity, state: new Map() }),
).resolves.toEqual(expect.objectContaining({ ok: false }));
});
});
plugins/catalog-backend/src/next/processing/DefaultCatalogProcessingOrchestrator.ts
+27 -2
View File
@@ -21,8 +21,9 @@ import {
LocationSpec,
parseLocationReference,
stringifyEntityRef,
stringifyLocationReference,
} from '@backstage/catalog-model';
import { ConflictError, InputError } from '@backstage/errors';
import { ConflictError, InputError, NotAllowedError } from '@backstage/errors';
import { ScmIntegrationRegistry } from '@backstage/integration';
import path from 'path';
import { Logger } from 'winston';
@@ -45,6 +46,7 @@ import {
validateEntity,
validateEntityEnvelope,
} from './util';
import { CatalogRulesEnforcer } from '../../ingestion/CatalogRules';
type Context = {
entityRef: string;
@@ -63,6 +65,7 @@ export class DefaultCatalogProcessingOrchestrator
logger: Logger;
parser: CatalogProcessorParser;
policy: EntityPolicy;
rulesEnforcer: CatalogRulesEnforcer;
},
) {}
@@ -117,8 +120,30 @@ export class DefaultCatalogProcessingOrchestrator
}
entity = await this.runPostProcessStep(entity, context);
// Check that any emitted entities are permitted to originate from that
// particular location according to the catalog rules
const collectorResults = context.collector.results();
for (const deferredEntity of collectorResults.deferredEntities) {
if (
!this.options.rulesEnforcer.isAllowed(
deferredEntity.entity,
context.originLocation,
)
) {
throw new NotAllowedError(
`Entity ${stringifyEntityRef(
deferredEntity.entity,
)} at ${stringifyLocationReference(
context.location,
)}, originated at ${stringifyLocationReference(
context.originLocation,
)}, is not of an allowed kind for that location`,
);
}
}
return {
...context.collector.results(),
...collectorResults,
completedEntity: entity,
state: new Map(),
ok: true,
plugins/catalog-backend/src/service/CatalogBuilder.ts
+2 -2
View File
@@ -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({