[Config Service] Expose `serverless` contextRef #156837

afharo · 2023-05-05T12:07:54Z

Summary

We need to expose some configuration to be available only when Kibana is running in Serverless offering. This PR adds the schema.contextRef('serverless') for developers to validate their settings based on this new context.

This PR also extends some documentation around the schema.contextRef API and uses the new context reference in the Elasticsearch service to define the defaults in elasticsearch.ignoreVersionMismatch.

Checklist

Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios

Risk Matrix

Risk	Probability	Severity	Mitigation/Notes
Developers might overcome the lack of `isServerless` API by setting a conditional config entry based on the new `serverless` context reference	Low	Low	If that ever happens, we'll discuss with them their needs.

For maintainers

This was checked for breaking API changes and was labeled appropriately

github-actions · 2023-05-05T12:08:08Z

Documentation preview:

✨ Changed pages

afharo · 2023-05-05T12:08:59Z

packages/kbn-config/src/config_service.ts

@@ -241,6 +241,7 @@ export class ConfigService {
      {
        dev: this.env.mode.dev,
        prod: this.env.mode.prod,
+        serverless: this.env.cliArgs.serverless === true,


It's optional, so I thought it's safer to validate it's true.

afharo · 2023-05-05T12:09:45Z

packages/core/test-helpers/core-test-helpers-kbn-server/src/create_root.ts

+    startKibana: async (abortSignal?: AbortSignal) => {
+      const root = createRootWithCorePlugins(kbnSettings, cliArgs);
+
+      abortSignal?.addEventListener('abort', async () => await root.shutdown());


I needed this so I could stop Kibana when left hanging waiting for a valid ES connection

afharo · 2023-05-05T12:10:28Z

packages/core/elasticsearch/core-elasticsearch-server-internal/src/elasticsearch_service.ts

@@ -97,8 +97,7 @@ export class ElasticsearchService
    const esNodesCompatibility$ = pollEsNodesVersion({
      internalClient: this.client.asInternalUser,
      log: this.log,
-      ignoreVersionMismatch:
-        config.ignoreVersionMismatch || this.coreContext.env.cliArgs.serverless === true,
+      ignoreVersionMismatch: config.ignoreVersionMismatch,


Back to not checking explicitly for serverless in this part of the code... 💆

afharo · 2023-05-05T12:11:00Z

docs/developer/architecture/core/configuration-service.asciidoc

+    enabled: schema.boolean({ defaultValue: schema.contextRef('dev') }),
+
+    // Setting only allowed in the Serverless offering
+    plansForWorldPeace: schema.conditional(


I was tempted with plansForWorldDomination, but I toned it down a bit 😅

afharo · 2023-05-05T12:12:58Z

packages/core/elasticsearch/core-elasticsearch-server-internal/src/elasticsearch_config.ts

+      // When running in serverless mode, default to `true`
+      defaultValue: schema.contextRef('serverless'),


I thought about nesting another conditional but found this solution a bit cleaner. Happy to iterate if you think it's best to have the explicit conditional.

This is fine as long as we're absolutely certain there aren't any other valid use cases where the version mismatch doesn't matter. If we're not certain, then a nested conditional is safer.

Yeah, fine for now ihmo, we can change later if needed.

afharo · 2023-05-05T12:13:52Z

src/core/server/integration_tests/elasticsearch/client.test.ts

@@ -68,7 +68,6 @@ function createFakeElasticsearchServer() {
  return server;
 }

-// FLAKY: https://github.com/elastic/kibana/issues/129754


left over: no longer flaky

elasticmachine · 2023-05-05T16:03:20Z

Pinging @elastic/kibana-core (Team:Core)

afharo · 2023-05-05T16:26:21Z

@elasticmachine merge upstream

jbudz

packages/kbn-test LGTM

TinaHeiligers

The implementation looks fine. I added a couple of comments/questions I'd like your input on first please.

TinaHeiligers · 2023-05-07T15:35:47Z

src/core/server/integration_tests/elasticsearch/version_compatibility.test.ts

+    });
+    await Promise.race([
+      firstValueFrom(found$),
+      startServers({ customKibanaVersion: nextMinor() }).then(() => {


This test only asserts against version mismatch between x.n and x.n+1. We should test against a mismatch between x.n and x.n-1 too.

Can we assume mismatches between current and previous/next patches won't matter?

This test only asserts against version mismatch between x.n and x.n+1. We should test against a mismatch between x.n and x.n-1 too

Kibana "is compatible" with newer minor versions of ES already:

kibana/packages/core/elasticsearch/core-elasticsearch-server-internal/src/version_check/es_kibana_version_compatability.ts

Lines 29 to 39 in 3508350

// Reject mismatching major version numbers.

if (esVersionNumbers.major !== kibanaVersionNumbers.major) {

return false;

}

// Reject older minor versions of ES.

if (esVersionNumbers.minor < kibanaVersionNumbers.minor) {

return false;

}

return true;

(which is unitary tested in the associated test file), so I'd say it's fine to only test the other scenario?

I guess it's good to have an integration test for the "ES is next minor" use case to catch any potential regressions in the future.

I added it in d4e619a (#156837)

Thank you for raising it, @TinaHeiligers

pgayvallet · 2023-05-08T08:07:27Z

docs/developer/architecture/core/configuration-service.asciidoc

+    plansForWorldPeace: schema.conditional(
+      schema.contextRef('serverless'),
+      true,
+      schema.string({ defaultValue: 'Kill all humans' }),


NIT: We should probably avoid that kind of word / sentence in our repo :)

Good point! I toned it down here: 859e3e1 (#156837)

pgayvallet · 2023-05-08T08:09:10Z

packages/core/elasticsearch/core-elasticsearch-server-internal/src/elasticsearch_config.ts

+      // When running in serverless mode, default to `true`
+      defaultValue: schema.contextRef('serverless'),


Yeah, fine for now ihmo, we can change later if needed.

pgayvallet · 2023-05-08T08:15:08Z

src/core/server/integration_tests/elasticsearch/version_compatibility.test.ts

+    });
+    await Promise.race([
+      firstValueFrom(found$),
+      startServers({ customKibanaVersion: nextMinor() }).then(() => {


This test only asserts against version mismatch between x.n and x.n+1. We should test against a mismatch between x.n and x.n-1 too

Kibana "is compatible" with newer minor versions of ES already:

kibana/packages/core/elasticsearch/core-elasticsearch-server-internal/src/version_check/es_kibana_version_compatability.ts

Lines 29 to 39 in 3508350

// Reject mismatching major version numbers.

if (esVersionNumbers.major !== kibanaVersionNumbers.major) {

return false;

}

// Reject older minor versions of ES.

if (esVersionNumbers.minor < kibanaVersionNumbers.minor) {

return false;

}

return true;

(which is unitary tested in the associated test file), so I'd say it's fine to only test the other scenario?

afharo · 2023-05-08T10:03:55Z

@elasticmachine merge upstream

kibana-ci · 2023-05-08T11:05:27Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 855b67a

Failed CI Steps

FTR Configs #27

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`@kbn/core-test-helpers-kbn-server`	48	49	+1

Unknown metric groups

API count

id	before	after	diff
`@kbn/core-test-helpers-kbn-server`	54	55	+1

ESLint disabled line counts

id	before	after	diff
`enterpriseSearch`	19	21	+2
`securitySolution`	398	401	+3
total			+5

Total ESLint disabled count

id	before	after	diff
`enterpriseSearch`	20	22	+2
`securitySolution`	478	481	+3
total			+5

History

💚 Build #125922 succeeded de65297
💔 Build #125894 failed ea011c9
💔 Build #125852 failed 414cb69b1dc2b05ef3ab550529c9997f4875d398
💔 Build #125819 failed 0998c4c18e0fb068e8d00d985db555dbe4c9b3de

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @afharo

## 📓 Summary Closes #157042 After the implementation of #156837, we are now able to limit the `logs.app_target` configuration to be accepted only in a serverless environment, otherwise, it'll not be accepted. Since we still need to use a default fallback value for the logs UI app, the fallback will be applied internally when looking for this config, to keep the original behaviour unchanged. ## 🧪 Testing ### Serverless environment - Start Elasticsearch with `yarn es snapshot` and Kibana with `yarn serverless-oblt` - Verify that when navigating to Logs pages through the sidebar links or directly accessing the url `/app/logs` it redirects to discover. ### Normal environment - Start Elasticsearch with `yarn es snapshot` and Kibana with `yarn start` - Verify that when navigating to Logs pages through the sidebar links or directly accessing the url `/app/logs` the log stream is rendered correctly. ### Limited to serverless env To verify that the configuration is only accepted under a serverless context, set into your kibana.yml or `kibana.dev.yml` the configuration ```yml xpack.infra.logs.app_target: discover ``` and verify that it triggers a validation error when running Kibana. --------- Co-authored-by: Marco Antonio Ghiani <[email protected]> Co-authored-by: Kibana Machine <[email protected]>

afharo self-assigned this May 5, 2023

afharo commented May 5, 2023

View reviewed changes

afharo force-pushed the config-service/expose-serverless-context branch from 0998c4c to 414cb69 Compare May 5, 2023 13:35

[Config Service] Expose serverless contextRef

ea011c9

afharo force-pushed the config-service/expose-serverless-context branch from 414cb69 to ea011c9 Compare May 5, 2023 15:17

afharo marked this pull request as ready for review May 5, 2023 16:03

afharo requested review from a team as code owners May 5, 2023 16:03

Merge branch 'main' into config-service/expose-serverless-context

de65297

jbudz approved these changes May 5, 2023

View reviewed changes

TinaHeiligers reviewed May 7, 2023

View reviewed changes

pgayvallet approved these changes May 8, 2023

View reviewed changes

afharo added 2 commits May 8, 2023 11:58

Add test for compatible "ES is next minor"

d4e619a

Tone down the defaultValue in the docs

859e3e1

Merge branch 'main' into config-service/expose-serverless-context

855b67a

afharo merged commit ff69433 into elastic:main May 8, 2023

afharo deleted the config-service/expose-serverless-context branch May 8, 2023 15:39

kibanamachine added the v8.9.0 label May 8, 2023

weltenwort mentioned this pull request May 8, 2023

Limit the target_app setting to serverless deployments #157042

Closed

stratoula mentioned this pull request May 11, 2023

[Serverless] Visualizations team changes #154307

Closed

legrego mentioned this pull request May 22, 2023

[Serverless] Disable UI of Users, Roles, and Role Mappings #158186

Merged

stratoula mentioned this pull request May 23, 2023

[tsvb] read only mode #157920

Merged

tonyghiani mentioned this pull request May 30, 2023

[Logs UI] Limit target_app config usage to serverless env #158649

Merged

azasypkin mentioned this pull request Jun 2, 2023

[APM] Serverless Onboarding with Custom Tutorials #158228

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Config Service] Expose `serverless` contextRef #156837

[Config Service] Expose `serverless` contextRef #156837

afharo commented May 5, 2023

github-actions bot commented May 5, 2023

afharo May 5, 2023

afharo May 5, 2023

afharo May 5, 2023

afharo May 5, 2023

afharo May 5, 2023

TinaHeiligers May 7, 2023

pgayvallet May 8, 2023

afharo May 5, 2023

elasticmachine commented May 5, 2023

afharo commented May 5, 2023

jbudz left a comment

TinaHeiligers left a comment

TinaHeiligers May 7, 2023

pgayvallet May 8, 2023

afharo May 8, 2023

pgayvallet May 8, 2023

afharo May 8, 2023

pgayvallet May 8, 2023

pgayvallet May 8, 2023

afharo commented May 8, 2023

kibana-ci commented May 8, 2023

API count

ESLint disabled line counts

Total ESLint disabled count

		// When running in serverless mode, default to `true`
		defaultValue: schema.contextRef('serverless'),

	// Reject mismatching major version numbers.
	if (esVersionNumbers.major !== kibanaVersionNumbers.major) {
	return false;
	}

	// Reject older minor versions of ES.
	if (esVersionNumbers.minor < kibanaVersionNumbers.minor) {
	return false;
	}

	return true;

[Config Service] Expose serverless contextRef #156837

[Config Service] Expose serverless contextRef #156837

Conversation

afharo commented May 5, 2023

Summary

Checklist

Risk Matrix

For maintainers

github-actions bot commented May 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented May 5, 2023

afharo commented May 5, 2023

jbudz left a comment

Choose a reason for hiding this comment

TinaHeiligers left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afharo commented May 8, 2023

kibana-ci commented May 8, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Public APIs missing comments

API count

ESLint disabled line counts

Total ESLint disabled count

History

[Config Service] Expose `serverless` contextRef #156837

[Config Service] Expose `serverless` contextRef #156837