catalog-backend: switch catalog rules to deny by default

plugins/catalog-backend/src/ingestion/CatalogRules.test.ts

@@ -45,11 +45,11 @@ const location: Record<string, LocationSpec> = {
 };
 
 describe('CatalogRulesEnforcer', () => {
-  it('should allow by default', () => {
+  it('should deny by default', () => {
     const enforcer = new CatalogRulesEnforcer([]);
-    expect(enforcer.isAllowed(entity.user, location.x)).toBe(true);
-    expect(enforcer.isAllowed(entity.group, location.y)).toBe(true);
-    expect(enforcer.isAllowed(entity.component, location.z)).toBe(true);
+    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);
   });
 
   it('should deny all', () => {
@@ -59,8 +59,10 @@ describe('CatalogRulesEnforcer', () => {
     expect(enforcer.isAllowed(entity.component, location.z)).toBe(false);
   });
 
-  it('should allow all with override', () => {
-    const enforcer = new CatalogRulesEnforcer([{ allow: [] }, { deny: [] }]);
+  it('should allow all', () => {
+    const enforcer = new CatalogRulesEnforcer([
+      { allow: [{ kind: 'User' }, { kind: 'Group' }, { kind: 'Component' }] },
+    ]);
     expect(enforcer.isAllowed(entity.user, location.x)).toBe(true);
     expect(enforcer.isAllowed(entity.group, location.y)).toBe(true);
     expect(enforcer.isAllowed(entity.component, location.z)).toBe(true);
@@ -68,7 +70,7 @@ describe('CatalogRulesEnforcer', () => {
 
   it('should deny groups', () => {
     const enforcer = new CatalogRulesEnforcer([
-      { allow: [], deny: [{ kind: 'Group' }] },
+      { allow: [{ kind: 'User' }, { kind: 'Component' }] },
     ]);
     expect(enforcer.isAllowed(entity.user, location.x)).toBe(true);
     expect(enforcer.isAllowed(entity.group, location.x)).toBe(false);
@@ -79,7 +81,8 @@ describe('CatalogRulesEnforcer', () => {
 
   it('should deny groups from github', () => {
     const enforcer = new CatalogRulesEnforcer([
-      { allow: [], deny: [{ kind: 'Group' }], locations: [{ type: 'github' }] },
+      { allow: [{ kind: 'User' }, { kind: 'Component' }] },
+      { allow: [{ kind: 'Group' }], locations: [{ type: 'file' }] },
     ]);
     expect(enforcer.isAllowed(entity.user, location.x)).toBe(true);
     expect(enforcer.isAllowed(entity.group, location.x)).toBe(false);
@@ -88,27 +91,26 @@ describe('CatalogRulesEnforcer', () => {
     expect(enforcer.isAllowed(entity.component, location.z)).toBe(true);
   });
 
-  it('should override to allow groups from files', () => {
+  it('should allow groups from files', () => {
     const enforcer = new CatalogRulesEnforcer([
-      { allow: [], deny: [{ kind: 'Group' }] },
-      { allow: [{ kind: 'Group' }], deny: [], locations: [{ type: 'file' }] },
+      { allow: [{ kind: 'Group' }], locations: [{ type: 'file' }] },
     ]);
-    expect(enforcer.isAllowed(entity.user, location.x)).toBe(true);
+    expect(enforcer.isAllowed(entity.user, location.x)).toBe(false);
     expect(enforcer.isAllowed(entity.group, location.x)).toBe(false);
     expect(enforcer.isAllowed(entity.group, location.y)).toBe(false);
     expect(enforcer.isAllowed(entity.group, location.z)).toBe(true);
-    expect(enforcer.isAllowed(entity.component, location.z)).toBe(true);
+    expect(enforcer.isAllowed(entity.component, location.z)).toBe(false);
   });
 
   it('should not be sensitive to kind case', () => {
     const enforcer = new CatalogRulesEnforcer([
-      { allow: [], deny: [{ kind: 'group' }] },
-      { allow: [], deny: [{ kind: 'Component' }] },
+      { allow: [{ kind: 'group' }] },
+      { allow: [{ kind: 'Component' }] },
     ]);
-    expect(enforcer.isAllowed(entity.user, location.x)).toBe(true);
-    expect(enforcer.isAllowed(entity.group, location.x)).toBe(false);
-    expect(enforcer.isAllowed(entity.group, location.y)).toBe(false);
-    expect(enforcer.isAllowed(entity.group, location.z)).toBe(false);
-    expect(enforcer.isAllowed(entity.component, location.z)).toBe(false);
+    expect(enforcer.isAllowed(entity.user, location.x)).toBe(false);
+    expect(enforcer.isAllowed(entity.group, location.x)).toBe(true);
+    expect(enforcer.isAllowed(entity.group, location.y)).toBe(true);
+    expect(enforcer.isAllowed(entity.group, location.z)).toBe(true);
+    expect(enforcer.isAllowed(entity.component, location.z)).toBe(true);
   });
 });

plugins/catalog-backend/src/ingestion/CatalogRules.ts

@@ -38,8 +38,7 @@ type LocationMatcher = {
  * An undefined list of matchers means match all, an empty list of matchers means match none
  */
 type CatalogRule = {
-  deny?: EntityMatcher[];
-  allow?: EntityMatcher[];
+  allow: EntityMatcher[];
   locations?: LocationMatcher[];
 };
 
@@ -51,8 +50,7 @@ export class CatalogRulesEnforcer {
    */
   static readonly defaultRules: CatalogRule[] = [
     {
-      deny: [{ kind: 'User' }, { kind: 'Group' }],
-      allow: [],
+      allow: [{ kind: 'Component' }, { kind: 'API' }],
     },
   ];
 
@@ -60,8 +58,10 @@ export class CatalogRulesEnforcer {
    * Loads catalog rules from config.
    *
    * This reads `catalog.rules` and defaults to the default rules if no value is present.
-   * The value of the config should be a list of config objects, each with a single `deny`
-   * field which in turn is a list of entity kind to deny.
+   * The value of the config should be a list of config objects, each with a single `allow`
+   * field which in turn is a list of entity kinds to allow.
+   *
+   * If there is no matching rule to allow an ingested entity, it will be rejected by the catalog.
    *
    * It also reads in rules from `catalog.locations`, where each location can have a list
    * of allowed entity for the location, specified in an `allow` field.
@@ -71,7 +71,7 @@ export class CatalogRulesEnforcer {
    * ```yaml
    * catalog:
    *   rules:
-   *   - deny: [User, Group, System]
+   *   - allow: [Component, API]
    *
    *   locations:
    *   - type: github
@@ -87,8 +87,7 @@ export class CatalogRulesEnforcer {
 
     if (config.has('catalog.rules')) {
       const globalRules = config.getConfigArray('catalog.rules').map(sub => ({
-        deny: sub.getStringArray('deny').map(kind => ({ kind })),
-        allow: [],
+        allow: sub.getStringArray('allow').map(kind => ({ kind })),
       }));
       rules.push(...globalRules);
     } else {
@@ -105,7 +104,6 @@ export class CatalogRulesEnforcer {
 
           return [
             {
-              deny: [],
               allow: sub.getStringArray('allow').map(kind => ({ kind })),
               locations: [
                 {
@@ -130,22 +128,17 @@ export class CatalogRulesEnforcer {
    * according to the configured rules.
    */
   isAllowed(entity: Entity, location: LocationSpec) {
-    let result = true;
-
     for (const rule of this.rules) {
       if (!this.matchLocation(location, rule.locations)) {
         continue;
       }
 
       if (this.matchEntity(entity, rule.allow)) {
-        result = true;
-      }
-      if (this.matchEntity(entity, rule.deny)) {
-        result = false;
+        return true;
       }
     }
 
-    return result;
+    return false;
   }
 
   private matchLocation(