Skip to content
Open
Show file tree
Hide file tree
Changes from 6 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
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