Support models declared by plugins

[vojtechszocs]

This PR ensures that Console is aware of any additional static models declared by plugins.

Overview

Plugins are encouraged to keep all of their model definitions in a single module, e.g. models.ts placed next to plugin.ts.

// models.ts

import { K8sKind } from '@console/internal/module/k8s';

export const FooBarModel: K8sKind = {
  apiGroup: 'test.io',
  apiVersion: 'v1alpha1',
  kind: 'FooBar',
  label: 'Foo Bar',
  labelPlural: 'Foo Bars',
  path: 'foobars',
  plural: 'foobars',
  abbr: 'FOOBAR',
  namespaced: true,
  id: 'foobar',
  crd: true,
};

To add these models, use the new ModelDefinition extension:

// plugin.ts

import * as _ from 'lodash-es';
import { Plugin, ModelDefinition } from '@console/plugin-sdk';
import * as models from './models';

type ConsumedExtensions = ModelDefinition /* | OtherExtension | ... */;

const plugin: Plugin<ConsumedExtensions> = [
  {
    type: 'ModelDefinition',
    properties: {
      models: _.values(models),
    },
  },
];

Browser console log:

Active plugins: [@console/demo-plugin]
1 new models added by plugins

The @console/active-plugins module codegen has been modified to export ActivePlugin[], i.e. the list of extensions enhanced with additional data:

export type ActivePlugin = {
  name: string;                 // = pkg.name
  extensions: Extension<any>[]; // = pkg.consolePlugin.entry
};

Any future metadata (e.g. plugin order) can be part of the ActivePlugin representation.

Note: from plugin author perspective, a plugin remains to be a list of Extension objects.

Handling model conflicts

A plugin cannot redefine existing models, this includes both Console base models and models declared previously by other plugins.

// models.ts

import { K8sKind } from '@console/internal/module/k8s';

export const FooBarModel: K8sKind = {
  apiGroup: 'test.io',
  apiVersion: 'v1alpha1',
  kind: 'FooBar',
  label: 'Foo Bar',
  labelPlural: 'Foo Bars',
  path: 'foobars',
  plural: 'foobars',
  abbr: 'FOOBAR',
  namespaced: true,
  id: 'foobar',
  crd: true,
};

// this model is conflicting with existing Console declaration:
// referenceForModel(baseModel) === referenceForModel(thisModel)
export const PrometheusModel: K8sKind = {
  kind: 'Prometheus',
  label: 'Prometheus',
  labelPlural: 'Prometheuses',
  apiGroup: 'monitoring.coreos.com',
  apiVersion: 'v1',
  path: 'prometheusi', // <-- change here
  abbr: 'PI',
  namespaced: true,
  crd: true,
  plural: 'prometheuses',
  propagationPolicy : 'Foreground',
};

The conflicting models will be reported in browser console log:

Active plugins: [@console/demo-plugin]
attempt to redefine model monitoring.coreos.com~v1~Prometheus
1 new models added by plugins

---

@mareklibra @jtomasek YAML template extension will be done in a separate PR.

[vojtechszocs]

/cc @spadgett @alecmerdler @christianvogt

[vojtechszocs]

@jtomasek This should fix the error you've had regarding Console not picking up your custom models, e.g. BaremetalHostModel.

[vojtechszocs]

The first two commits are from other un-merged PRs, please review the last one.

[spadgett]

Do we expect multiple plugins to define the same model? I feel like we should at least print a warning, particularly if the models aren't exactly the same.

[vojtechszocs]

That's a good point.

A plugin should not re-define existing static models, this includes both core Console models (definitions in public/models/index.ts) and models contributed by other plugins (e.g. BaremetalHostModel from Metal3 plugin).

I feel like we should at least print a warning, particularly if the models aren't exactly the same.

Agreed, I'll adapt the code accordingly.

[vojtechszocs]

PR updated, conflicting models will be discarded along with a console log warning.

[vojtechszocs]

@spadgett Please check out PR description for an overview of recent changes.

As discussed earlier, the concept of plugin registry module will go away in favor of HOC to connect relevant React components to extensions they're interested in.

[vojtechszocs]

Note - this is an implication of eslint-env node above.

[vojtechszocs]

As discussed earlier, I've adapted function declarations to const since the latter form seems to be preferred.

[christianvogt]

@vojtechszocs I do not understand why models are treated differently than any other extension. To me this sets a bad precedence for others to start defining extensions in various other ways.

[vojtechszocs]

@vojtechszocs I do not understand why models are treated differently than any other extension. To me this sets a bad precedence for others to start defining extensions in various other ways.

My motivation was to establish a convention where each plugin has a module (src/models.ts) that exports all static model definitions.

But you're right. Models are just another type of Console extension and shouldn't be treated differently.

I'll change the ActivePlugin type to:

export type ActivePlugin = {
  name: string;                 // = pkg.name
  extensions: Extension<any>[]; // = pkg.consolePlugin.entry
};

and introduce new extension type for models. Plugins can still have src/models.ts and use import * as syntax to pull all models together.

[vojtechszocs]

PR updated according to @christianvogt's comment.

[christianvogt]

/lgtm

This looks much better now @vojtechszocs
What's the policy on adding more console noise as I see a couple of console.info calls during normal flows.

[jtomasek]

/lgtm
Tested with metal3 plugin (#1539), works as expected.

[spadgett]

/approve

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: christianvogt, jtomasek, spadgett, vojtechszocs

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• frontend/OWNERS [spadgett]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[vojtechszocs]

What's the policy on adding more console noise as I see a couple of console.info calls during normal flows.

No such policy that I'm aware of, it's a good point for discussion, cc @spadgett.

Production builds could have some (info, warn) logs removed, call signature of console global could be restricted to only {info,warn,error}. Development builds could retain all the logs.