backstage: 🐛 Bug Report: Modifying Entity Schema Defined In BuiltinKindsEntityProcessor

📜 Description

We are looking to adjust the Domain entity kind and adjust the owner field to be optional. The docs note this use case, but the builder.replaceProcessors() does not allow replacing or modifying the BuiltinKindsEntityProcessor.

👍 Expected behavior

It should allow for replacement or adjustment of the kinds processed by BuiltinKindsEntityProcessor.

👎 Actual Behavior with Screenshots

The BuiltinKindsEntityProcessor validation throws an error as the Entity does not match the default schema, and the Entity will not continue to any custom Entity processors further in the list.

Processor BuiltinKindsEntityProcessor threw an error while validating the entity domain:default/finance; caused by TypeError: /spec must have required property 'owner' - missingProperty: owner type=plugin entity=domain:default/finance. location=file:/~/backstage/fixtures/domain-finance.yaml

👟 Reproduction steps

A Domain without an owner such as the following will fail the default schema validation check as expected.

apiVersion: backstage.io/v1beta1
kind: Domain
metadata:
  name: finance
spec: {}

Adding in a custom Domain Entity processor such as the following where domainEntityV1alpha1Validator sets the spec schema "required": [].

import { Entity, entityKindSchemaValidator } from '@backstage/catalog-model';
import { CatalogProcessor } from '@backstage/plugin-catalog-node';
import { Logger } from 'winston';
import domainSchema from './domainEntityV1alpha1Validator';

export class DomainCustomEntityProcessor implements CatalogProcessor {
  getProcessorName(): string {
    return 'DomainCustomEntityProcessor';
  }

  async validateEntityKind(entity: Entity): Promise<boolean> {
    const validate = entityKindSchemaValidator(domainSchema);
    return validate(entity) === entity;
  }
}

Using this in the backend plugin catalog createPlugin, the replaceProcessor() does not affect the built-in Entities.

  const defaultProcessors = builder.getDefaultProcessors();
  builder.replaceProcessors([
    new DomainCustomEntityProcessor({ logger }),
    ...defaultProcessors
  ]);

📃 Provide the context for the Bug.

There seems to be prior art that was moving towards enabling this such as #7834. The docs imply that, while high risk, the capability should exist. In v1.9.0, the validation step was adjusted to continue through all registered processors.

The adjustment wasn’t enough to enable this use case though as the BuiltInKindsEntityProcessor throws an error due to the failed validation and further processors are not run. The replaceProcessors() only affects some of the processors, and PlaceholderProcessor and BuiltinKindsEntityProcessor are hardcoded first in the processor list.

🖥️ Your Environment

No response

👀 Have you spent some time to check if this bug has been raised before?

I checked and didn’t find similar issue

🏢 Have you read the Code of Conduct?

I have read the Code of Conduct

Are you willing to submit PR?

None

About this issue

Original URL
State: closed
Created a year ago
Comments: 16 (14 by maintainers)

Most upvoted comments

Probably best to leave as is for now. As you’ve seen we do have exports for apiEntityV1alpha1Validator etc which helps in making the processor a lot smaller, but there’s no similar thing for the relations.

I think this might make sense to defer until we have a great pattern for the catalog backend in terms of the new backend system. Maybe we’ll end up packaging up the kinds as individual modules for the catalog so you can install them at will, one at a time or all of them at the same time. There should probably be a @backstage/plugin-catalog-backend-module-system-model or something like that, which contains all of the default system model parts nicely packaged up. Then we can start thinking about steering away from the catalog builder entirely, and new ways of composing these changes.

freben on Mar 22, 2023

One more thing. There’s nothing stopping you from using delegation / composition as a pattern, right?

Completely untested but maybe conveys the idea:

class CustomKinds implements CatalogProcessor {
  readonly #delegate = new BuiltinKindsEntityProcessor();
  
  getProcessorName(): string {
    return this.#delegate.getProcessorName();
  }

  async validateEntityKind(entity: Entity): Promise<boolean> {
    if (entity.kind === 'System') {
      // custom code
    } else {
      return this.#delegate.validateEntityKind(entity);
    }
  }

  // and the same for postProcessEntity

freben on Mar 22, 2023

Yep it takes the shortcut of not checking the apiVersion indeed, that should also be fixed at the same time.

freben on Mar 22, 2023

Yeah, indeed that’s an omission right now; you can’t replace that particular processor in its entirety.

What I could envision adding, while not super beautiful, is an if statement in the catalog builder here, which skips adding that processor, if the user has supplied a processor that returns the exact same name. Would you like to make such a pull request? 🙏

Ping @backstage/catalog-core here too.

freben on Mar 16, 2023