🐛 Tech Insights: Incorrect results due to check merging

[hspens]

Workspace

tech-insights

📜 Description

Unexpected results are returned from runChecks in @backstage-community/plugin-tech-insights-backend-module-jsonfc since ^0.5.0.

It seems that checks using a fact with a FactSchema identical to another fact are "merged".

👍 Expected behavior

Checks should only run for entities using facts with a matching entityFilter.

👎 Actual Behavior with Screenshots

Facts matching mutually exclusive entities are still used if their fact schemas are identical.

Screenshot shows 2 entities and 2 checks.

One entity has a lifecycle: experimental and one has lifecycle: production. There are 2 different facts defined, both validate hasOwner but have different entityFilters. One with spec.lifecycle: experimental and one with spec.lifecycle: production. The facts have the same fact schema.

👟 Reproduction steps

Steps to Reproduce:

1. Run yarn install to install dependencies.
2. Duplicate the example-website entity in existing examples (use different lifecycles)
3. Duplicate the entityOwnershipFactRetriever fact retriever and add 'spec.lifecycle': 'production' to the entityFilter.
4. Duplicate the groupOwnerCheck check and adjust the factIds of that check
5. Run yarn start to start the development server.

Example entities

apiVersion: backstage.io/v1alpha1
kind: Component
metadata:
  name: example-website-experimental
spec:
  type: website
  lifecycle: experimental
  owner: guests
  system: examples
  providesApis: [example-grpc-api]
---
apiVersion: backstage.io/v1alpha1
kind: Component
metadata:
  name: example-website-production
spec:
  type: website
  lifecycle: production
  owner: guests
  system: examples
  providesApis: [example-grpc-api]

Fact Retrievers

Experimental

export const entityOwnershipExperimentalFactRetriever: FactRetriever = {
  id: 'entityOwnershipExperimentalFactRetriever',
  version: '0.0.1',
  title: 'Entity Ownership in Experimental',
  description:
    'Generates facts which indicate the quality of data in the spec.owner field',
  entityFilter: [
    {
      kind: ['component', 'domain', 'system', 'api', 'resource', 'template'],
      'spec.lifecycle': 'experimental',
    },
  ],
  schema: {
    hasOwner: {
      type: 'boolean',
      description: 'The spec.owner field is set',
    },
    hasGroupOwner: {
      type: 'boolean',
      description: 'The spec.owner field is set and refers to a group',
    },
  },
...

Production

export const entityOwnershipProductionFactRetriever: FactRetriever = {
  id: 'entityOwnershipProductionFactRetriever',
  version: '0.0.1',
  title: 'Entity Ownership in Production',
  description:
    'Generates facts which indicate the quality of data in the spec.owner field',
  entityFilter: [
    {
      kind: ['component', 'domain', 'system', 'api', 'resource', 'template'],
      'spec.lifecycle': 'production',
    },
  ],
  schema: {
    hasOwner: {
      type: 'boolean',
      description: 'The spec.owner field is set',
    },
    hasGroupOwner: {
      type: 'boolean',
      description: 'The spec.owner field is set and refers to a group',
    },
  },
...

Checks

export const checks = [
  {
    id: 'groupOwnerCheckExperimental',
    type: JSON_RULE_ENGINE_CHECK_TYPE,
    name: 'Group Owner Check Experimental',
    description:
      'Verifies that a Group has been set as the owner for this entity (lifecycle: experimental)',
    factIds: ['entityOwnershipExperimentalFactRetriever'],
    rule: {
      conditions: {
        all: [
          {
            fact: 'hasGroupOwner',
            operator: 'equal',
            value: true,
          },
        ],
      },
    },
  },
  {
    id: 'groupOwnerCheckProduction',
    type: JSON_RULE_ENGINE_CHECK_TYPE,
    name: 'Group Owner Check Production',
    description:
      'Verifies that a Group has been set as the owner for this entity (lifecycle: production)',
    factIds: ['entityOwnershipProductionFactRetriever'],
    rule: {
      conditions: {
        all: [
          {
            fact: 'hasGroupOwner',
            operator: 'equal',
            value: true,
          },
        ],
      },
    },
  }
]

app-config.yaml

techInsights:
  factRetrievers:
    entityOwnershipExperimentalFactRetriever:
      cadence: '*/2 * * * *'
      lifecycle: { timeToLive: { weeks: 2 } }
    entityOwnershipProductionFactRetriever:
      cadence: '*/2 * * * *'
      lifecycle: { timeToLive: { weeks: 2 } }

---

It seems like it starts to work when changing

const hasFacts = usedFacts.every(factId =>
  factValues.hasOwnProperty(factId),
);

to

const hasFacts = techInsightCheck.factIds.every(factId => facts[factId]);

in plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.ts, (runChecks).

📃 Provide the context for the Bug.

We have a "migration" page on Backstage displaying progress of change. Some of these migrations are supported by facts that are identical in terms of their schema but uses different entity filters. After upgrading to ^0.5.0 of @backstage-community/plugin-tech-insights-backend-module-jsonfc we saw the progress of certain migrations change as multiple different, mutually exclusive, facts/checks ran for entities not matching their entity filter.

👀 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

[awanlin]

Thanks @hspens, happy to have contributions to correct this!

@Pike @Xantier any thoughts on this as owners?

[Pike]

Thanks @hspens, happy to have contributions to correct this!

@Pike @Xantier any thoughts on this as owners?

@punkle instead

[awanlin]

Sorry @Pike thanks for the correction!