Skip to content
Open
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
5 changes: 0 additions & 5 deletions x-pack/platform/packages/shared/kbn-change-history/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,3 @@ export type * from './src/types';
export * from './src/client';
export { CHANGE_HISTORY_AGGREGATE_FIELDS, LEGACY_CHANGE_HISTORY_ACTION_IDS } from './src/types';
export { DEFAULT_FIELD_AGGREGATION_SIZE } from './src/constants';
/**
* @internal exported for test use only — do NOT use in production code,
* this could cause the index to be created before the feature is ready for GA
*/
export { FLAGS } from './src/constants';
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { loggingSystemMock } from '@kbn/core-logging-server-mocks';
import { ToolingLog } from '@kbn/tooling-log';
import type { EsTestCluster } from '@kbn/test';
import { createTestEsCluster } from '@kbn/test';
import { FLAGS } from '../src/constants';
import { ChangeHistoryClient } from '..';
import { DATA_STREAM_NAME } from '../src/client';
import type { LogChangeHistoryOptions, ObjectChange } from '..';
Expand Down Expand Up @@ -47,7 +46,6 @@ describe('ChangeHistoryClient', () => {
};

beforeAll(async () => {
FLAGS.FEATURE_ENABLED = true;
jest.setTimeout(30_000);
esServer = createTestEsCluster({
log: new ToolingLog({ writeTo: process.stdout, level: 'debug' }),
Expand All @@ -58,7 +56,6 @@ describe('ChangeHistoryClient', () => {

afterAll(async () => {
await esServer.stop();
FLAGS.FEATURE_ENABLED = false;
});

afterEach(async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { elasticsearchServiceMock } from '@kbn/core/server/mocks';
import { loggingSystemMock } from '@kbn/core-logging-server-mocks';
import { DataStreamClient } from '@kbn/data-streams';
import { withSpan } from '@kbn/apm-utils';
import { FLAGS } from './constants';
import { ChangeHistoryClient } from './client';
import type { ObjectChange } from './types';

Expand Down Expand Up @@ -48,7 +47,6 @@ describe('ChangeHistoryClient', () => {
};

beforeEach(() => {
FLAGS.FEATURE_ENABLED = true;
DataStreamClientMock.initialize.mockResolvedValue(dataStreamClientMock as never);
});

Expand Down Expand Up @@ -435,7 +433,7 @@ describe('ChangeHistoryClient.logBulk', () => {
};

beforeEach(() => {
FLAGS.FEATURE_ENABLED = true;
DataStreamClientMock.initialize.mockResolvedValue({} as never);
});

afterEach(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import type { ClientCreateRequest } from '@kbn/data-streams/src/types/es_api';
import type { Logger } from '@kbn/logging';
import { changeHistoryMappings } from './mappings';
import {
FLAGS,
DATA_STREAM_NAME,
SEPARATOR_CHAR,
ECS_VERSION,
Expand Down Expand Up @@ -116,11 +115,6 @@ export class ChangeHistoryClient implements IChangeHistoryClient {
* @throws An error if the data stream is not initialized properly.
*/
async initialize(elasticsearchClient: ElasticsearchClient) {
if (!FLAGS.FEATURE_ENABLED) {
const error = new Error(`Change history is disabled. Skipping initialization.`);
this.logger.error(error);
throw error;
}
const definition: DataStreamDefinition<typeof changeHistoryMappings.v1, ChangeHistoryDocument> =
{
name: DATA_STREAM_NAME,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,3 @@ export const DEFAULT_RESULT_SIZE = 100;
* Default maximum number of buckets returned per field by {@link ChangeHistoryClient.getHistoryByFields}.
*/
export const DEFAULT_FIELD_AGGREGATION_SIZE = 100;

/**
* Acts like a feature flag for this package as it prevents initialization.
* Remove this after General Availability
* */
export const FLAGS = {
FEATURE_ENABLED: true,
};
Original file line number Diff line number Diff line change
Expand Up @@ -105,18 +105,15 @@ export async function logRuleChanges({

const ruleType = getRuleType(ruleTypeRegistry, ruleSO.attributes.alertTypeId, logger);

// "ruleType.trackChanges" is activated at Alerting plugin's "plugin.ts".
//
// The activation is gated by the feature flag "xpack.alerting.ruleChangeTracking.enabled".
// On top of that "xpack.alerting.ruleChangeTracking.scope" controls what solution rule
// types will be activated, e.g. "security" or "observability".
//
// "ruleType.trackChanges" is activated at Alerting plugin's "plugin.ts", based on
// "xpack.alerting.ruleChangeTracking.scope", which controls what solution rule
// types are activated, e.g. "security" or "observability".
if (!ruleType?.trackChanges) {
continue;
}

// Security Solution additionally gates rule changes history per-space via its
// "Enable rule changes history" advanced setting, on top of the static config flag above.
// "Enable rule changes history" advanced setting.
if (ruleType.solution === 'security') {
if (securityRuleChangesHistoryEnabled === undefined) {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ describe('config validation', () => {
"removalDelay": "1h",
},
"ruleChangeTracking": Object {
"enabled": true,
"scope": Array [
"security",
],
Expand Down
1 change: 0 additions & 1 deletion x-pack/platform/plugins/shared/alerting/server/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ export const configSchema = schema.object({
coordinateInstallation: schema.boolean({ defaultValue: true }),
}),
ruleChangeTracking: schema.object({
enabled: schema.boolean({ defaultValue: true }),
scope: schema.arrayOf(ruleChangeTrackingSolutions, { defaultValue: ['security'] }),
Comment thread
maximpn marked this conversation as resolved.
Comment thread
maximpn marked this conversation as resolved.
Comment thread
kibanamachine marked this conversation as resolved.
}),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Removing the enabled key from the ruleChangeTracking schema without a config deprecation is an upgrade-safety regression. schema.object rejects unknown keys by default, and there's no matching entry in config_deprecations.ts, so any deployment that still has xpack.alerting.ruleChangeTracking.enabled: false (the opt-out this PR is removing) in kibana.yml will hit a fatal config validation error on startup after upgrade — including on the 9.5/9.6 backport branches.

The PR description already identifies this risk. The standard fix is to register an unused() deprecation so the stale key is dropped with a warning instead of crashing:

// config_deprecations.ts
unused('ruleChangeTracking.enabled', {
  level: 'warning',
  message: 'The setting "xpack.alerting.ruleChangeTracking.enabled" is deprecated and no longer used. Please remove it.',
}),

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good general rule, but I don't think it applies here. ruleChangeTracking.enabled was only ever introduced in #261981 (2026-04-28), after the 9.5 branch was cut (2026-04-10) — it's not present on the 9.4 branch at all, and 9.5.0 hasn't GA'd yet (no v9.5.0 tag; current release train is 9.4, up to v9.4.3).

So there's no released Kibana version where a real deployment could have set xpack.alerting.ruleChangeTracking.enabled: false in their kibana.yml — the key only ever existed on unreleased 9.5/main dev builds. Since this PR removes it before 9.5.0 ships, no customer upgrade path is affected, so I don't think we need an unused() deprecation for it.

cancelAlertsOnRuleTimeout: schema.boolean({ defaultValue: true }),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,23 @@ const applyConfigDeprecations = (settings = {}) => {
};

describe('config deprecations', () => {
it('removes xpack.alerting.ruleChangeTracking.enabled', async () => {
const config = {
ruleChangeTracking: {
enabled: false,
scope: ['security'],
},
};
const { messages, migrated } = applyConfigDeprecations(cloneDeep(config));
expect(migrated.ruleChangeTracking?.enabled).not.toBeDefined();
expect(migrated.ruleChangeTracking?.scope).toEqual(['security']);
expect(messages).toMatchInlineSnapshot(`
Array [
"You no longer need to configure \\"ruleChangeTracking.enabled\\".",
]
`);
});

it('renames xpack.alerting.maintenanceWindow.enabled to xpack.maintenanceWindows.enabled', async () => {
const config = {
xpack: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import type { ConfigDeprecationProvider } from '@kbn/core/server';
export const autocompleteConfigDeprecationProvider: ConfigDeprecationProvider = ({
renameFromRoot,
deprecate,
unused,
}) => [
unused('ruleChangeTracking.enabled', { level: 'warning' }),
deprecate('maxEphemeralActionsPerAlert', '9.0.0', {
level: 'warning',
message: `The setting "xpack.alerting.maxEphemeralActionsPerAlert" is deprecated and currently ignored by the system. Please remove this setting.`,
Expand Down
4 changes: 1 addition & 3 deletions x-pack/platform/plugins/shared/alerting/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -281,9 +281,7 @@ export class AlertingPlugin {
this.disabledRuleTypes = new Set(this.config.disabledRuleTypes || []);
this.enabledRuleTypes =
this.config.enabledRuleTypes != null ? new Set(this.config.enabledRuleTypes) : null;
if (this.config.ruleChangeTracking.enabled) {
this.changeTrackingService = new ChangeTrackingService(this.logger, this.kibanaVersion);
}
this.changeTrackingService = new ChangeTrackingService(this.logger, this.kibanaVersion);
}

public setup(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ describe('createRuleRoute', () => {
},
cancelAlertsOnRuleTimeout: true,
ruleChangeTracking: {
enabled: false,
scope: ['security'] as string[],
},
rules: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ import type { RuleChangeHistorySnapshot } from '../lib/change_tracking';
import { getRuleSo } from '../../data/rule';

/**
* Thrown by {@link RulesClient.getHistory} when rule change tracking is
* disabled at the framework level (`xpack.alerting.ruleChangeTracking.enabled = false`).
* Thrown by {@link RulesClient.getHistory} when the change tracking service is unavailable.
*/
export class RuleChangeTrackingDisabledError extends Error {
constructor(message = 'Rule change tracking is disabled.') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ export function generateAlertingConfig(overwrites: Partial<AlertingConfig> = {})
coordinateInstallation: true,
},
ruleChangeTracking: {
enabled: false,
scope: ['security'],
},
invalidateApiKeysTask: {
Expand Down
11 changes: 4 additions & 7 deletions x-pack/platform/test/alerting_api_integration/common/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ interface CreateTestConfigOptions {
maxAlerts?: number;
emailMaximumBodyLength?: number;
indexRefreshInterval?: string | false;
ruleChangeTrackingEnabled?: boolean;
ruleChangeTrackingScope?: string[];
}

// test.not-enabled is specifically not enabled
Expand Down Expand Up @@ -236,7 +236,7 @@ export function createTestConfig(name: string, options: CreateTestConfigOptions)
experimentalFeatures = [],
maxAlerts = 20,
indexRefreshInterval,
ruleChangeTrackingEnabled = false,
ruleChangeTrackingScope,
} = options;

return async ({ readConfigFile }: FtrConfigProviderContext) => {
Expand Down Expand Up @@ -332,11 +332,8 @@ export function createTestConfig(name: string, options: CreateTestConfigOptions)
? []
: [`--xpack.actions.email.maximum_body_length=${options.emailMaximumBodyLength}`];

const ruleChangeTrackingSettings = ruleChangeTrackingEnabled
? [
'--xpack.alerting.ruleChangeTracking.enabled=true',
`--xpack.alerting.ruleChangeTracking.scope=${JSON.stringify(['stack'])}`,
]
const ruleChangeTrackingSettings = ruleChangeTrackingScope
? [`--xpack.alerting.ruleChangeTracking.scope=${JSON.stringify(ruleChangeTrackingScope)}`]
: [];

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ dependsOn:
- '@kbn/alerts-as-data-utils'
- '@kbn/data-plugin'
- '@kbn/zod'
- '@kbn/change-history'
tags:
- plugin
- prod
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import type { IEventLogClientService, IEventLogService } from '@kbn/event-log-pl
import type { NotificationsPluginStart } from '@kbn/notifications-plugin/server';
import { RULE_SAVED_OBJECT_TYPE } from '@kbn/alerting-plugin/server';
import { ALERTING_FEATURE_ID } from '@kbn/alerting-plugin/common';
import { FLAGS as CHANGE_HISTORY_FLAGS } from '@kbn/change-history';
import { defineRoutes } from './routes';
import { defineActionTypes } from './action_types';
import { defineRuleTypes } from './rule_types';
Expand Down Expand Up @@ -112,7 +111,6 @@ export class FixturePlugin implements Plugin<void, void, FixtureSetupDeps, Fixtu
core: CoreSetup<FixtureStartDeps>,
{ features, actions, alerting, taskManager, ruleRegistry, eventLog }: FixtureSetupDeps
) {
CHANGE_HISTORY_FLAGS.FEATURE_ENABLED = true;
features.registerKibanaFeature({
id: 'alertsFixture',
name: 'Alerts',
Expand Down Expand Up @@ -228,7 +226,4 @@ export class FixturePlugin implements Plugin<void, void, FixtureSetupDeps, Fixtu
this.notificationsStart$.next(notifications);
this.notificationsStart$.complete();
}
public stop() {
CHANGE_HISTORY_FLAGS.FEATURE_ENABLED = false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
"@kbn/alerts-as-data-utils",
"@kbn/data-plugin",
"@kbn/zod",
"@kbn/change-history",
],
"exclude": [
"target/**/*",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export default function changeTrackingEnabledTest({ getService }: FtrProviderCon
const retry = getService('retry');

describe('change tracking service - enabled', () => {
it('should create the change history data stream when ruleChangeTracking is enabled', async () => {
it('should create the change history data stream for stack-scoped rule types', async () => {
const client = asKibanaClient(es);
await retry.tryForTime(30_000, async () => {
const response = await client.indices.getDataStream({ name: '.kibana_change_history' });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export default createTestConfig('spaces_only', {
enableActionsProxy: false,
verificationMode: 'none',
useDedicatedTaskRunner: false,
ruleChangeTrackingEnabled: true,
ruleChangeTrackingScope: ['stack'],
testFiles: [require.resolve('./change_tracking/enabled.ts')],
reportName: 'X-Pack Alerting API Integration Tests - Change Tracking Enabled',
});
Original file line number Diff line number Diff line change
Expand Up @@ -321,16 +321,6 @@ export const allowedExperimentalValues = Object.freeze({
*/
prebuiltRulesDeprecationUIEnabled: true,

/**
* Enables the Detection Rule Changes History API endpoint
* (`GET /api/detection_engine/rules/_history`).
*
* Independent of the alerting framework's `xpack.alerting.ruleChangeTracking.enabled`
* config flag, which gates the underlying primitive that produces the history
* records. Both must be enabled for the API to return non-empty results.
*/
ruleChangesHistoryEnabled: true,

/**
* Enables the agent builder `run_rule_preview` tool and the `security.rule.preview`
* attachment (server type + client renderer). Gates registration so the feature can
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,6 @@ import { useManualRuleRunConfirmation } from '../../../rule_gaps/components/manu
// eslint-disable-next-line no-restricted-imports
import { useLegacyUrlRedirect } from './use_redirect_legacy_url';
import { RuleDetailTabs, useRuleDetailsTabs } from './use_rule_details_tabs';
import { useIsExperimentalFeatureEnabled } from '../../../../common/hooks/use_experimental_features';
import { useRuleUpdateCallout } from '../../../rule_management/hooks/use_rule_update_callout';
import { useDeprecatedRuleDetailsCallout } from '../../../rule_management/components/rule_deprecation';
import { useUserPrivileges } from '../../../../common/components/user_privileges';
Expand Down Expand Up @@ -235,14 +234,9 @@ export const RuleDetailsPage = connector(
clearEventsLoading,
clearSelected,
}: DetectionEngineComponentProps) {
const ruleChangesHistoryFFEnabled = useIsExperimentalFeatureEnabled(
'ruleChangesHistoryEnabled'
);
const [ruleChangesHistoryAdvancedSetting] = useUiSetting$<boolean>(
const [isRuleChangesHistoryEnabled] = useUiSetting$<boolean>(
ENABLE_RULE_CHANGES_HISTORY_SETTING
);
const isRuleChangesHistoryEnabled =
ruleChangesHistoryFFEnabled && ruleChangesHistoryAdvancedSetting;

const {
application,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import {
getRuleChangesHistoryUrl,
} from '../../../../../common/components/link_to/redirect_to_detection_engine';
import { useBoolState } from '../../../../../common/hooks/use_bool_state';
import { useIsExperimentalFeatureEnabled } from '../../../../../common/hooks/use_experimental_features';
import { SINGLE_RULE_ACTIONS } from '../../../../../common/lib/apm/user_actions';
import { useStartTransaction } from '../../../../../common/lib/apm/use_start_transaction';
import { useKibana, useUiSetting$ } from '../../../../../common/lib/kibana';
Expand Down Expand Up @@ -106,12 +105,7 @@ const RuleActionsOverflowComponent = ({
state: { doesBaseVersionExist },
} = useRuleCustomizationsContext();

const ruleChangesHistoryFFEnabled = useIsExperimentalFeatureEnabled('ruleChangesHistoryEnabled');
const [ruleChangesHistoryAdvancedSetting] = useUiSetting$<boolean>(
ENABLE_RULE_CHANGES_HISTORY_SETTING
);
const isRuleChangesHistoryEnabled =
ruleChangesHistoryFFEnabled && ruleChangesHistoryAdvancedSetting;
const [isRuleChangesHistoryEnabled] = useUiSetting$<boolean>(ENABLE_RULE_CHANGES_HISTORY_SETTING);

const actions = useMemo(
() => [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,12 +202,7 @@ const RulesContainerComponent: React.FC = () => {
const isEndpointExceptionsMovedFFEnabled = useIsExperimentalFeatureEnabled(
'endpointExceptionsMovedUnderManagement'
);
const ruleChangesHistoryFFEnabled = useIsExperimentalFeatureEnabled('ruleChangesHistoryEnabled');
const [ruleChangesHistoryAdvancedSetting] = useUiSetting$<boolean>(
ENABLE_RULE_CHANGES_HISTORY_SETTING
);
const isRuleChangesHistoryEnabled =
ruleChangesHistoryFFEnabled && ruleChangesHistoryAdvancedSetting;
const [isRuleChangesHistoryEnabled] = useUiSetting$<boolean>(ENABLE_RULE_CHANGES_HISTORY_SETTING);

const subRoutes = useMemo(() => {
return getRulesSubRoutes(capabilities, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,7 @@ export const registerRuleManagementRoutes = (
// Rules coverage overview
getCoverageOverviewRoute(router);

// Rule changes history (gated by experimental flag; the feature also
// requires `xpack.alerting.ruleChangeTracking.enabled` to be on for the
// alerting framework to actually produce history records).
if (config.experimentalFeatures.ruleChangesHistoryEnabled) {
ruleHistoryRoute(router);
restoreRuleFromHistoryRoute(router);
}
// Rule changes history
ruleHistoryRoute(router);
restoreRuleFromHistoryRoute(router);
};
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ describe('initUiSettings', () => {
enableAlertsAndAttacksAlignment: false,
extendedRuleExecutionLoggingEnabled: false,
newFlyoutSystemDisabled: false,
ruleChangesHistoryEnabled: false,
} as ExperimentalFeatures;

beforeEach(() => {
Expand Down
Loading
Loading