Kibana: Fail startup with unknown optionalPlugins dependency in dev mode

Created on 8 Apr 2020  路  7Comments  路  Source: elastic/kibana

Currently you can run Kibana completely fine if you specify an unknown/invalid optionalPlugins id in your kibana.json file. This make totally sense in production mode, but I'd suggest failing startup in dev mode for that.

Why? Plugin ids are not at all type-safe, meaning if we change the id of a plugin, it can easily be forgotten to update it in other plugins that just have this as optionalPlugins, since we might not even be aware of those other plugins, plus if a functional test for that specific feature is missing it will very easily slip all CI tests. (This e.g. happened here: https://github.com/elastic/kibana/pull/62543#issuecomment-610041905)

I think it would make sense to fail Kibana startup during dev mode if we specify an optionalPlugin id that doesn't exist. That way we could catch those errors directly during development, and also functional tests (which run in dev mode??) would fail and the CI wouldn't let it pass.

Alternatively if this solution doesn't sound feasible for reasons I've missed so far, could we have some kind of other CI check that validates all dependencies ids for their validity?

Plugins Core

Most helpful comment

I think it's really valuable to have little to no behavior differences between:

  • Prod and dev
  • 1st and 3rd party plugins

If we want to validate this for internal development, I suggest a script that runs on CI to read all the kibana.json files and verifies that the optional and required dependencies exist in the repository.

All 7 comments

Pinging @elastic/kibana-platform (Team:Platform)

Why? Plugin ids are not at all type-safe, meaning if we change the id of a plugin, it can easily be forgotten to update it in other plugins that just have this as optionalPlugins, since we might not even be aware of those other plugins, plus if a functional test for that specific feature is missing it will very easily slip all CI tests

In an ideal world, this would be covered by FTR tests running a plugin and it's (optional) dependencies and asserting the dependencies-related features of the plugin are working as expected.

The https://github.com/elastic/kibana/pull/62543#issuecomment-610041905 reference you linked showed a test failing due to a pluginId reference being forgotten / not renamed, which seems to be what we want and how we should detect that imho.

I can understand that ATM every feature is not necessarily properly covered by FTR tests, and therefor this safety net could avoid a lot of developer errors during migration, but OTOH this would be introducing a significant behavioral difference between dev and prod environments.

Would like to see the other team members' opinion on that one.

and also functional tests (which run in dev mode??)

Someone would need to confirm that, but I think it's running in production mode (at least it should, as it does when FTR is running against could instances). I don't see any option setting the env to dev for serverArgs in test/common/config.js

But is there any legit use-case where we would want to have (inside the core Kibana repository) optional plugin dependencies to IDs that are not existing?

I think it's really valuable to have little to no behavior differences between:

  • Prod and dev
  • 1st and 3rd party plugins

If we want to validate this for internal development, I suggest a script that runs on CI to read all the kibana.json files and verifies that the optional and required dependencies exist in the repository.

If we want to validate this for internal development, I suggest a script that runs on CI to read all the kibana.json files and verifies that the optional and required dependencies exist in the repository.

That seems like a very reasonable solution. @timroes wdyt?

I agree with that. My really mainly want to fail CI. So as long as CI fails if we use it, I am fine :-)

Should we also check for missing dependencies on test plugins (ie test/plugin_functional/plugins/*/kibana.json)?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

stacey-gammon picture stacey-gammon  路  3Comments

MaartenUreel picture MaartenUreel  路  3Comments

tbragin picture tbragin  路  3Comments

timmolter picture timmolter  路  3Comments

Ginja picture Ginja  路  3Comments