Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Config Service] Expose serverless contextRef #156837

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions docs/developer/architecture/core/configuration-service.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -147,3 +147,64 @@ export const config: PluginConfigDescriptor<ConfigType> = {
],
};
----
[[validating-your-configuration-based-on-context-references]]
=== Validating your configuration based on context references
Some features require special configuration when running in different modes (dev/prod/dist, or even serverless). For purpose, core injects the following _references_ in the validation's context:

[cols="^1,^1,3"]
|===
|Context Reference |Potential values |Description

|`dev`
|`true`\|`false`
|Is Kibana running in Dev mode?

|`prod`
|`true`\|`false`
|Is Kibana running in Production mode (running from binary)?

|`dist`
|`true`\|`false`
|Is Kibana running from a distributable build (not running from source)?

|`serverless`
|`true`\|`false`
|Is Kibana running in Serverless offering?

|`version`
|`8.9.0`
|The current version of Kibana

|`buildNum`
|`12345`
|The build number

|`branch`
|`main`
|The current branch running

|`buildSha`
|`12345`
|The build SHA (typically refers to the last commit's SHA)

|===

To use any of the references listed above in a config validation schema, they can be accessed via `schema.contextRef('{CONTEXT_REFERENCE}')`:

[source,js]
----
export const config = {
schema: schema.object({
// Enabled by default in Dev mode
enabled: schema.boolean({ defaultValue: schema.contextRef('dev') }),

// Setting only allowed in the Serverless offering
plansForWorldPeace: schema.conditional(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

schema.contextRef('serverless'),
true,
schema.string({ defaultValue: 'Free hugs' }),
schema.never()
),
}),
};
----
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,8 @@ export const configSchema = schema.object({
return '"ignoreVersionMismatch" can only be set to true in development mode';
}
},
defaultValue: false,
// When running in serverless mode, default to `true`
defaultValue: schema.contextRef('serverless'),
Comment on lines +157 to +158
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}),
schema.boolean({ defaultValue: false })
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

esVersionCheckInterval: config.healthCheckDelay.asMilliseconds(),
kibanaVersion: this.kibanaVersion,
}).pipe(takeUntil(this.stop$), shareReplay({ refCount: true, bufferSize: 1 }));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ export interface TestKibanaUtils {

export interface TestUtils {
startES: () => Promise<TestElasticsearchUtils>;
startKibana: () => Promise<TestKibanaUtils>;
startKibana: (abortSignal?: AbortSignal) => Promise<TestKibanaUtils>;
}

/**
Expand Down Expand Up @@ -261,7 +261,7 @@ export function createTestServers({
// Add time for KBN and adding users
adjustTimeout(es.getStartTimeout() + 100000);

const kbnSettings = settings.kbn ?? {};
const { cliArgs = {}, customKibanaVersion, ...kbnSettings } = settings.kbn ?? {};

return {
startES: async () => {
Expand All @@ -284,8 +284,10 @@ export function createTestServers({
password: kibanaServerTestUser.password,
};
},
startKibana: async () => {
const root = createRootWithCorePlugins(kbnSettings);
startKibana: async (abortSignal?: AbortSignal) => {
const root = createRootWithCorePlugins(kbnSettings, cliArgs, customKibanaVersion);

abortSignal?.addEventListener('abort', async () => await root.shutdown());
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


await root.preboot();
const coreSetup = await root.setup();
Expand Down
1 change: 1 addition & 0 deletions packages/kbn-config/src/config_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ export class ConfigService {
{
dev: this.env.mode.dev,
prod: this.env.mode.prod,
serverless: this.env.cliArgs.serverless === true,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

...this.env.packageInfo,
},
`config validation of [${namespace}]`
Expand Down
2 changes: 1 addition & 1 deletion packages/kbn-test/src/es/test_es_cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export interface CreateTestEsClusterOptions {
clusterName?: string;
/**
* Path to data archive snapshot to run Elasticsearch with.
* To prepare the the snapshot:
* To prepare the snapshot:
* - run Elasticsearch server
* - index necessary data
* - stop Elasticsearch server
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ function createFakeElasticsearchServer() {
return server;
}

// FLAKY: https://github.com/elastic/kibana/issues/129754
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left over: no longer flaky

describe('fake elasticsearch', () => {
let esServer: http.Server;
let kibanaServer: Root;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import {
createTestServers,
type TestElasticsearchUtils,
type TestKibanaUtils,
} from '@kbn/core-test-helpers-kbn-server';
import { esTestConfig } from '@kbn/test';
import { firstValueFrom, Subject } from 'rxjs';
import { CliArgs } from '@kbn/config';
import Semver from 'semver';

function nextMinor() {
return Semver.inc(esTestConfig.getVersion(), 'minor') || '10.0.0';
}

function previousMinor() {
const [major, minor] = esTestConfig
.getVersion()
.split('.')
.map((s) => parseInt(s, 10));
// We should be fine for now. When we jump to the next major, we'll need to handle that.
return `${major}.${minor - 1}.0`;
}

describe('Version Compatibility', () => {
let esServer: TestElasticsearchUtils | undefined;
let kibanaServer: TestKibanaUtils | undefined;
let abortController: AbortController | undefined;
let consoleSpy: jest.SpyInstance;

beforeEach(() => {
consoleSpy = jest.spyOn(console, 'log');
});

afterEach(async () => {
consoleSpy.mockRestore();
if (kibanaServer) {
await kibanaServer.stop();
} else {
abortController?.abort();
}
if (esServer) {
await esServer.stop();
}
kibanaServer = undefined;
abortController = undefined;
esServer = undefined;
});

const startServers = async ({
cliArgs,
customKibanaVersion,
ignoreVersionMismatch,
}: {
cliArgs?: Partial<CliArgs>;
customKibanaVersion?: string;
ignoreVersionMismatch?: boolean;
} = {}) => {
const { startES, startKibana } = createTestServers({
adjustTimeout: jest.setTimeout,
settings: {
kbn: {
cliArgs,
customKibanaVersion,
elasticsearch: {
ignoreVersionMismatch,
},
},
},
});

esServer = await startES();
abortController = new AbortController();
kibanaServer = await startKibana(abortController.signal);
};

it('should start when versions match', async () => {
await expect(startServers({})).resolves.toBeUndefined();
});

it('should start when ES is next minor', async () => {
await expect(startServers({ customKibanaVersion: previousMinor() })).resolves.toBeUndefined();
});

it('should flag the incompatibility on version mismatch (ES is previous minor)', async () => {
const found$ = new Subject<void>();
consoleSpy.mockImplementation((str) => {
if (str.includes('is incompatible')) {
found$.next();
}
});
await Promise.race([
firstValueFrom(found$),
startServers({ customKibanaVersion: nextMinor() }).then(() => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

// 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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

throw new Error(
'Kibana completed the bootstrap without finding the incompatibility message'
);
}),
new Promise((resolve, reject) =>
setTimeout(() => reject(new Error('Test timedout')), 5 * 60 * 1000)
),
]).finally(() => found$.complete());
});

it('should ignore the version mismatch when option is set', async () => {
await expect(
startServers({
customKibanaVersion: nextMinor(),
cliArgs: { dev: true },
ignoreVersionMismatch: true,
})
).resolves.toBeUndefined();
});

it('should not allow the option when not in dev mode', async () => {
await expect(
startServers({ customKibanaVersion: nextMinor(), ignoreVersionMismatch: true })
).rejects.toThrowErrorMatchingInlineSnapshot(
`"[config validation of [elasticsearch].ignoreVersionMismatch]: \\"ignoreVersionMismatch\\" can only be set to true in development mode"`
);
});

it('should ignore version mismatch when running on serverless mode and complete startup', async () => {
await expect(
startServers({ customKibanaVersion: nextMinor(), cliArgs: { serverless: true } })
).resolves.toBeUndefined();
});
});