Skip to content

Commit

Permalink
[Event Log] Skip setting assets to hidden in serverless (#164767)
Browse files Browse the repository at this point in the history
Resolves #163891.

In this PR, I'm making the event log skip converting existing assets to
hidden if it is in a serverless deployment. Given assets that need to be
converted to hidden would exist prior to 8.0, there isn't such a case in
serverless and we do not have the necessary legacy APIs to retrieve
them.

## To verify on Serverless
- Ensure on serverless there are no more warnings as indicated in
#163891.

## To verify on non-Serverless
1. Create a 7.15 deployment (and persisting the ES data so you can
upgrade later)
2. Create an alerting rule (with short interval, say 10s)
3. Let the alerting rule run a few times so it generates event logs
4. Upgrade ES to 7.17 (it will fail otherwise)
5. Upgrade your deployment to this PR
6. Check the event log index template, indices and alias for 7.15 to
ensure they got converted to hidden
- `curl -XGET
http://elastic:changeme@localhost:9200/_template/.kibana-event-log-7.15.3-template`
- `curl -XGET
http://elastic:changeme@localhost:9200/.kibana-event-log-7.15.3-000001`
- `curl -XGET
http://elastic:changeme@localhost:9200/_alias/.kibana-event-log-7.15.3`

---------

Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
mikecote and kibanamachine authored Aug 28, 2023
1 parent 0113eb7 commit 35b7b08
Show file tree
Hide file tree
Showing 9 changed files with 49 additions and 6 deletions.
3 changes: 2 additions & 1 deletion x-pack/plugins/event_log/kibana.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
"eventLog"
],
"optionalPlugins": [
"spaces"
"spaces",
"serverless"
]
}
}
1 change: 1 addition & 0 deletions x-pack/plugins/event_log/server/es/context.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const createContextMock = () => {
} = {
logger: loggingSystemMock.createLogger(),
esNames: namesMock.create(),
shouldSetExistingAssetsToHidden: true,
initialize: jest.fn(),
shutdown: jest.fn(),
waitTillReady: jest.fn(async () => true),
Expand Down
6 changes: 6 additions & 0 deletions x-pack/plugins/event_log/server/es/context.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ describe('createEsContext', () => {
test('should return is ready state as falsy if not initialized', () => {
const context = createEsContext({
logger,
shouldSetExistingAssetsToHidden: true,
indexNameRoot: 'test0',
kibanaVersion: '1.2.3',
elasticsearchClientPromise: Promise.resolve(elasticsearchClient),
Expand All @@ -48,6 +49,7 @@ describe('createEsContext', () => {
test('should return esNames', () => {
const context = createEsContext({
logger,
shouldSetExistingAssetsToHidden: true,
indexNameRoot: 'test-index',
kibanaVersion: '1.2.3',
elasticsearchClientPromise: Promise.resolve(elasticsearchClient),
Expand All @@ -65,6 +67,7 @@ describe('createEsContext', () => {
test('should return exist false for esAdapter index template and data stream before initialize', async () => {
const context = createEsContext({
logger,
shouldSetExistingAssetsToHidden: true,
indexNameRoot: 'test1',
kibanaVersion: '1.2.3',
elasticsearchClientPromise: Promise.resolve(elasticsearchClient),
Expand All @@ -86,6 +89,7 @@ describe('createEsContext', () => {
test('should return exist true for esAdapter index template and data stream after initialize', async () => {
const context = createEsContext({
logger,
shouldSetExistingAssetsToHidden: true,
indexNameRoot: 'test2',
kibanaVersion: '1.2.3',
elasticsearchClientPromise: Promise.resolve(elasticsearchClient),
Expand Down Expand Up @@ -117,6 +121,7 @@ describe('createEsContext', () => {

const context = createEsContext({
logger,
shouldSetExistingAssetsToHidden: true,
indexNameRoot: 'test2',
kibanaVersion: '1.2.3',
elasticsearchClientPromise: Promise.resolve(elasticsearchClient),
Expand All @@ -134,6 +139,7 @@ describe('createEsContext', () => {
jest.requireMock('./init').initializeEs.mockResolvedValue(false);
const context = createEsContext({
logger,
shouldSetExistingAssetsToHidden: true,
indexNameRoot: 'test2',
kibanaVersion: '1.2.3',
elasticsearchClientPromise: Promise.resolve(elasticsearchClient),
Expand Down
4 changes: 4 additions & 0 deletions x-pack/plugins/event_log/server/es/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export interface EsContext {
waitTillReady(): Promise<boolean>;
readonly initialized: boolean;
readonly retryDelay: number;
shouldSetExistingAssetsToHidden: boolean;
}

export interface EsError {
Expand All @@ -38,6 +39,7 @@ export interface EsContextCtorParams {
logger: Logger;
indexNameRoot: string;
kibanaVersion: string;
shouldSetExistingAssetsToHidden: boolean;
elasticsearchClientPromise: Promise<ElasticsearchClient>;
}

Expand All @@ -48,13 +50,15 @@ class EsContextImpl implements EsContext {
private readonly readySignal: ReadySignal<boolean>;
public initialized: boolean;
public readonly retryDelay: number;
public readonly shouldSetExistingAssetsToHidden: boolean;

constructor(params: EsContextCtorParams) {
this.logger = params.logger;
this.esNames = getEsNames(params.indexNameRoot, params.kibanaVersion);
this.readySignal = createReadySignal();
this.initialized = false;
this.retryDelay = RETRY_DELAY;
this.shouldSetExistingAssetsToHidden = params.shouldSetExistingAssetsToHidden;
this.esAdapter = new ClusterClientAdapter({
logger: params.logger,
elasticsearchClientPromise: params.elasticsearchClientPromise,
Expand Down
18 changes: 18 additions & 0 deletions x-pack/plugins/event_log/server/es/init.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,12 @@ describe('initializeEs', () => {
expect(esContext.esAdapter.setLegacyIndexTemplateToHidden).not.toHaveBeenCalled();
});

test(`should not read or update existing index templates when specifying shouldSetExistingAssetsToHidden=false`, async () => {
await initializeEs({ ...esContext, shouldSetExistingAssetsToHidden: false });
expect(esContext.esAdapter.getExistingLegacyIndexTemplates).not.toHaveBeenCalled();
expect(esContext.esAdapter.setLegacyIndexTemplateToHidden).not.toHaveBeenCalled();
});

test(`should continue initialization if getting existing index templates throws an error`, async () => {
esContext.esAdapter.getExistingLegacyIndexTemplates.mockRejectedValue(new Error('Fail'));

Expand Down Expand Up @@ -198,6 +204,12 @@ describe('initializeEs', () => {
expect(esContext.esAdapter.setIndexToHidden).not.toHaveBeenCalled();
});

test(`should not read or update existing index settings when specifying shouldSetExistingAssetsToHidden=false`, async () => {
await initializeEs({ ...esContext, shouldSetExistingAssetsToHidden: false });
expect(esContext.esAdapter.getExistingIndices).not.toHaveBeenCalled();
expect(esContext.esAdapter.setIndexToHidden).not.toHaveBeenCalled();
});

test(`should continue initialization if getting existing index settings throws an error`, async () => {
esContext.esAdapter.getExistingIndices.mockRejectedValue(new Error('Fail'));

Expand Down Expand Up @@ -287,6 +299,12 @@ describe('initializeEs', () => {
expect(esContext.esAdapter.setIndexAliasToHidden).not.toHaveBeenCalled();
});

test(`should not read or update existing index aliases when specifying shouldSetExistingAssetsToHidden=false`, async () => {
await initializeEs({ ...esContext, shouldSetExistingAssetsToHidden: false });
expect(esContext.esAdapter.getExistingIndexAliases).not.toHaveBeenCalled();
expect(esContext.esAdapter.setIndexAliasToHidden).not.toHaveBeenCalled();
});

test(`should continue initialization if getting existing index aliases throws an error`, async () => {
esContext.esAdapter.getExistingIndexAliases.mockRejectedValue(new Error('Fail'));

Expand Down
7 changes: 5 additions & 2 deletions x-pack/plugins/event_log/server/es/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,11 @@ export async function initializeEs(esContext: EsContext): Promise<boolean> {
async function initializeEsResources(esContext: EsContext) {
const steps = new EsInitializationSteps(esContext);

// today, setExistingAssetsToHidden() never throws, but just in case ...
await retry(steps.setExistingAssetsToHidden);
// Only set existing assets to hidden if we're not in serverless
if (esContext.shouldSetExistingAssetsToHidden) {
// today, setExistingAssetsToHidden() never throws, but just in case ...
await retry(steps.setExistingAssetsToHidden);
}
await retry(steps.createIndexTemplateIfNotExists);
await retry(steps.createDataStreamIfNotExists);

Expand Down
6 changes: 4 additions & 2 deletions x-pack/plugins/event_log/server/plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ describe('event_log plugin', () => {
const coreStart = coreMock.createStart() as CoreStart;

const plugin = new Plugin(initializerContext);
const setup = plugin.setup(coreSetup);
// serverless setup is currently empty, and there is no mock
const setup = plugin.setup(coreSetup, { serverless: {} });
expect(typeof setup.getLogger).toBe('function');
expect(typeof setup.getProviderActions).toBe('function');
expect(typeof setup.isIndexingEntries).toBe('function');
Expand All @@ -40,7 +41,8 @@ describe('event_log plugin', () => {

const plugin = new Plugin(initializerContext);
const spaces = spacesMock.createStart();
plugin.setup(coreSetup);
// serverless setup is currently empty, and there is no mock
plugin.setup(coreSetup, { serverless: {} });
plugin.start(coreStart, { spaces });
await plugin.stop();
expect(mockLogger.debug).toBeCalledWith('shutdown: waiting to finish');
Expand Down
9 changes: 8 additions & 1 deletion x-pack/plugins/event_log/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
IClusterClient,
} from '@kbn/core/server';
import { SpacesPluginStart } from '@kbn/spaces-plugin/server';
import { ServerlessPluginSetup } from '@kbn/serverless/server';

import type {
IEventLogConfig,
Expand All @@ -35,6 +36,10 @@ const ACTIONS = {
stopping: 'stopping',
};

interface PluginSetupDeps {
serverless?: ServerlessPluginSetup;
}

interface PluginStartDeps {
spaces?: SpacesPluginStart;
}
Expand All @@ -56,7 +61,7 @@ export class Plugin implements CorePlugin<IEventLogService, IEventLogClientServi
this.kibanaVersion = this.context.env.packageInfo.version;
}

setup(core: CoreSetup): IEventLogService {
setup(core: CoreSetup, plugins: PluginSetupDeps): IEventLogService {
const kibanaIndex = core.savedObjects.getDefaultIndex();
this.systemLogger.debug('setting up plugin');

Expand All @@ -67,6 +72,8 @@ export class Plugin implements CorePlugin<IEventLogService, IEventLogClientServi
.getStartServices()
.then(([{ elasticsearch }]) => elasticsearch.client.asInternalUser),
kibanaVersion: this.kibanaVersion,
// Only non-serverless deployments may have assets that need to be converted
shouldSetExistingAssetsToHidden: !plugins.serverless,
});

this.eventLogService = new EventLogService({
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/event_log/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
"@kbn/utility-types",
"@kbn/std",
"@kbn/safer-lodash-set",
"@kbn/serverless",
],
"exclude": [
"target/**/*",
Expand Down

0 comments on commit 35b7b08

Please sign in to comment.