NP apps with custom appRoute fail to load due to missing legacy injected vars

[azasypkin]

Chromeless apps with custom appRoute fail to load because of missing legacy injected vars required by non-related new platform plugins.

Steps to reproduce:

1. Define the app:

core.application.register({
  appRoute: '/some',
  chromeless: true,
  id: 'some',
  title: 'some',
  mount({ element }) {
    function SomeComponent() {
      return <div>Some</div>;
    }
    render(<SomeComponent />, element);
    return () => {
      unmountComponentAtNode(element);
    };
  },
});

2. Define the custom app route:

router.get(
  { path: '/some', validate: false },
  async (context, request, response) => {
    return response.ok({
        body: await context.core.rendering.render({ includeUserSettings: false }),
        headers: { 'content-security-policy': csp.header },
      });
  })
);

3. Try to open http://localhost:5601/some in the browser.

Expected behavior:

View should load successfully.

Screenshots (if relevant):

Errors in browser console (if relevant):

Completely unrelated Newsfeed plugin causes fatal error due to missing injected var it depends on:

TypeError: "config is undefined"
    getApi api.ts:198
    fetchNewsfeed plugin.tsx:74
    start plugin.tsx:55
    _callee3$ plugin.ts:171
    tryCatch runtime.js:45
    invoke runtime.js:271
    method runtime.js:97
    asyncGeneratorStep plugin.ts:14
    _next plugin.ts:16
    _asyncToGenerator plugin.ts:16
    _asyncToGenerator plugin.ts:16
    start plugin.ts:187
    _callee2$ plugins_service.ts:249
    tryCatch runtime.js:45
    invoke runtime.js:271
    method runtime.js:97
    asyncGeneratorStep plugins_service.ts:12
    _next plugins_service.ts:14

/cc @eliperelman @legrego

[elasticmachine]

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

[pgayvallet]

So. The actual error cause is that when rendering a NP app, the injectedMetadata.vars is not populated, as this is an option that should be manually provided when calling

context.core.rendering.render({vars: [...]})

in the ui_render_mixin this is done using legacy API:

kibana/src/legacy/ui/ui_render/ui_render_mixin.js

Lines 185 to 193 in 357be59

	const vars = await legacy.getVars(app.getId(), h.request, {
	apmConfig: getApmConfig(app),
	...overrides,
	});
	const content = await rendering.render(h.request, uiSettings, {
	app,
	includeUserSettings,
	vars,
	});

But when rendering NP app, it seems there is no way to actually provide these vars to the rendering service.

However the newsfeed NP plugin (and probably others) does access these vars, which results later in an NPE:

kibana/src/plugins/newsfeed/public/plugin.tsx

Lines 55 to 59 in a50dbef

	private fetchNewsfeed(core: CoreStart) {
	const { http, injectedMetadata } = core;
	const config = injectedMetadata.getInjectedVar(
	'newsfeed'
	) as NewsfeedPluginInjectedConfig['newsfeed'];

I thought we had functional tests covering this, but TIL they are only checking the response status, missing any actual error when displaying the page...

kibana/test/api_integration/apis/core/index.js

Lines 37 to 40 in 054ec70

	it('provides access to application rendering client', async () => {
	await supertest.get('/requestcontext/render/core').expect(200, /app:core/);
	await supertest.get('/requestcontext/render/testbed').expect(200, /app:testbed/);
	});

It seems we need a way to retrieve the legacy vars the same way ui_render_mixin does, however I'm not sure what the best way is (or if that's the actual solution)

@eliperelman @joshdover will need your insight on this one.

[pgayvallet]

Also, as already mentioned with @azasypkin on slack, I think we should really provide an helper API to avoid plugins manually registering their custom route path handler on every app using an appRoute.

Instead of

    router.get(
      {
        path: '/my-app-route',
        validate: false,
      },
      async (context, req, res) => {
        // + additional parameters for vars it seems
        const body = await context.core.rendering.render({ includeUserSettings: false }); 

        return res.ok({
          body,
          headers: {
            'content-security-policy': core.http.csp.header,
          },
        });
      }
    );

should we just add something like

core.http.registerCustomAppRoute('/my-app-route', { includeUserSettings, requireAuth })

As it seems 99% (maybe 100%?) of the usages will be the exact same code snippet, that we could automate inside core.

Also for futur-us: there is a typo in the security header that needs to be fixed for FT when we will add them:

kibana/src/plugins/testbed/server/index.ts

Lines 95 to 101 in 56041f0

	return res.ok({
	body,
	headers: {
	'content-securty-policy': core.http.csp.header,
	},
	});
	}

[pgayvallet]

Also related to #54376

[joshdover]

It seems we need a way to retrieve the legacy vars the same way ui_render_mixin does, however I'm not sure what the best way is (or if that's the actual solution)

I think there's two options here:

1. Stop exposing the getInjected API to New Platform plugins. From what I understand there's only 2-3 plugins using this, and we plan to remove it anyways. It was only introduced as a stop-gap before Expose whitelisted config values to client-side plugin #50641 was introduced.
2. Move all the legacy logic into the New Platform for getting the injected vars.

(1) is seems much more preferable as it's work that needs to be done anyways, and we won't throw any of it away like we would with the work required for (2).

[pgayvallet]

I would like to think option 1 is the way to go, however the feature is currently broken, and going this path makes us depends on feature teams changes. I we decide to do this, would we handle the migration of getInjected-using code ourselves ?

For the newsfeed plugin, the migration is pretty trivial, just need to replace the exposed var to the new exposeToBrowser configuration.