[UA] Do not allow reindexing data_stream backing indices (and other enhancements) by gsoldevila · Pull Request #213755 · elastic/kibana

gsoldevila · 2025-03-10T14:34:02Z

Summary

The PR brings the following improvements to the UA ES deprecations flows:

Remove the "Reindex" option for Frozen index deprecations that are related to a data stream's backing index (backing indices cannot be alias-based, see thread).
Propagate "isClosedIndex" information to the UI side, so that we can show a warning message in the reindex flows flyouts. The original functionality got lost in the woods.
Propagate "isClosedIndex", "isFrozenIndex", "isReadonly" properties as part of the correctiveAction details ONLY for index_settings migrations, instead of doing it as top level properties for all migrations. These properties did not make for most migration types.
Big refactor in migrations.ts file (server side). The logic for obtaining, filtering and enriching ES deprecations had become quite fragmented and hard to read. @Bamieh @jloleysens please take a close look at this one.
Improve existing type definitions.

Here are screenshots for each of the combinations:

old + open

old + closed

old + frozen + open

old + frozen + closed

old + open + manual write block (readonly)

old + closed + manual write block (readonly)

old + frozen + open + manual write block (readonly)

old + frozen + closed + manual write block (readonly)

elasticmachine · 2025-03-10T14:34:05Z

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

Bamieh

Checked the PR again after the more recent changes. LTGM. The migrations file needed some refactoring :elasticheart:

jloleysens

Great work @gsoldevila !! Thanks for restoring the lost behaviour and cleaning up the code a bit, way more readable now 👏🏻

I did not test locally but the logic and re-added copy looks solid. When you get a chance, if you still have this running would you mind adding some screenshots to the PR description of what changed?

jloleysens · 2025-03-13T08:41:51Z

...s_deprecations/deprecation_types/indices/flyout/steps/details/unfreeze_details_step.test.tsx

+      />
+    );
+
+    expect(wrapper).toMatchInlineSnapshot(`


nit: I tried to get this right but I couldn't get the assertions to be against just the data-test-subj attribute (in this case data-test-subj="startIndexReadonlyButton"). I believe there should be a way to say: "I expect data tests subject X to exist".

These snapshots serve that purpose too, they just a bit verbose IMO.

Not something we need to change on this PR though!

jloleysens · 2025-03-13T08:42:23Z

...nts/es_deprecations/deprecation_types/indices/flyout/steps/details/unfreeze_details_step.tsx

                  </EuiText>
                ),
              },
+              /* We cannot unfreeze backing indices (that would break the related data_stream) */


Suggested change

/* We cannot unfreeze backing indices (that would break the related data_stream) */

/* We cannot reindex backing indices in the same way as regular indices (that would break the related data_stream) */

jloleysens · 2025-03-13T08:42:59Z

...nts/es_deprecations/deprecation_types/indices/flyout/steps/details/unfreeze_details_step.tsx

+                            id="xpack.upgradeAssistant.esDeprecations.indices.indexFlyout.detailsStep.unfreeze.option2.description"
+                            defaultMessage="Alternatively, you can reindex the data into a new, compatible index. All existing documents will be copied over to a new index, and the old index will be removed. Depending on the size of the index and the available resources, the reindexing operation can take some time. Your data will be in read-only mode until the reindexing has completed."
+                          />
+                          {isClosedIndex && (


Nice! This addresses my concern of showing this warning when the reindex option is not present.

jloleysens · 2025-03-13T08:45:01Z

...nts/es_deprecations/deprecation_types/indices/flyout/steps/details/unfreeze_details_step.tsx

          <EuiFlexItem grow={false}>
            <EuiFlexGroup gutterSize="s">
-              {!hasFetchFailed && !isCompleted && hasRequiredPrivileges && (
+              {/* We cannot unfreeze backing indices (that would break the related data_stream) */}


Suggested change

{/* We cannot unfreeze backing indices (that would break the related data_stream) */}

{/* We cannot reindex backing indices in the same way as regular indices (that would break the related data_stream) */}

…leanup

elasticmachine · 2025-03-13T13:23:04Z

💚 Build Succeeded

Buildkite Build
Commit: bc32d65

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`upgradeAssistant`	194	195	+1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`upgradeAssistant`	212.2KB	213.8KB	+1.6KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`upgradeAssistant`	23.8KB	23.8KB	-1.0B

History

gsoldevila · 2025-03-13T14:03:02Z

💚 All backports created successfully

Status	Branch	Result
✅	8.18

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

@Bamieh

…nhancements) (elastic#213755) ## Summary The PR brings the following improvements to the UA ES deprecations flows: * Remove the "Reindex" option for _Frozen index_ deprecations that are related to a data stream's backing index (backing indices cannot be alias-based, see [thread](https://elastic.slack.com/archives/C08A04N8XHV/p1741017934359239)). * Propagate `"isClosedIndex"` information to the UI side, so that we can show a warning message in the reindex flows flyouts. The [original functionality](elastic#58890) got lost in the woods. * Propagate `"isClosedIndex", "isFrozenIndex", "isReadonly"` properties as part of the `correctiveAction` details ONLY for _index_settings migrations_, instead of doing it as top level properties for all migrations. These properties did not make for most migration types. * Big refactor in `migrations.ts` file (server side). The logic for obtaining, filtering and enriching ES deprecations had become quite fragmented and hard to read. @Bamieh @jloleysens please take a close look at this one. * Improve existing type definitions. (cherry picked from commit f2ae059)

@Bamieh

…other enhancements) (#213755) (#214386) # Backport This will backport the following commits from `8.x` to `8.18`: - [[UA] Do not allow reindexing data_stream backing indices (and other enhancements) (#213755)](#213755)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)

gsoldevila added Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// release_note:skip Skip the PR/issue when compiling release notes backport:version Backport to applied version labels v8.18.0 v8.19.0 labels Mar 10, 2025

gsoldevila requested review from Bamieh and jloleysens March 10, 2025 14:34

gsoldevila requested a review from kibanamachine as a code owner March 10, 2025 14:34

gsoldevila marked this pull request as draft March 10, 2025 14:38

sabarasaba mentioned this pull request Mar 10, 2025

[UA] Dont show reindexing CTA if an index is a datastream index #213041

Closed

gsoldevila marked this pull request as ready for review March 11, 2025 14:11

Bamieh approved these changes Mar 11, 2025

View reviewed changes

gsoldevila force-pushed the no-reindexing-data-stream-backing-indices branch from 74bf706 to 0cd39ab Compare March 12, 2025 16:52

gsoldevila changed the title ~~Do not allow reindexing data_stream backing indices~~ [UA] Do not allow reindexing data_stream backing indices (and other enhancements) Mar 12, 2025

Bamieh approved these changes Mar 12, 2025

View reviewed changes

jloleysens approved these changes Mar 13, 2025

View reviewed changes

gsoldevila added 7 commits March 13, 2025 11:03

Do not allow reindexing data_stream backing indices

a31c1fe

Update old tests, add test for data stream backing indices

f3f58ac

Update outdated tests with the new isInDataStream property

10a0b1e

Exclude "reindex" migrations when the index is frozen, simplify and c…

d9946a7

…leanup

Propagate "isClosedIndex" to the UI. Misc code cleanup / refactor.

d066040

Fix types issues

841dbcb

Address PR feedback, fix tests

c775fd4

gsoldevila force-pushed the no-reindexing-data-stream-backing-indices branch from 933d08d to c775fd4 Compare March 13, 2025 10:03

Manually update translations

bc32d65

gsoldevila merged commit f2ae059 into elastic:8.x Mar 13, 2025
8 checks passed

gsoldevila mentioned this pull request Mar 13, 2025

[8.18] [UA] Do not allow reindexing data_stream backing indices (and other enhancements) (#213755) #214386

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[UA] Do not allow reindexing data_stream backing indices (and other enhancements)#213755

[UA] Do not allow reindexing data_stream backing indices (and other enhancements)#213755
gsoldevila merged 8 commits intoelastic:8.xfrom
gsoldevila:no-reindexing-data-stream-backing-indices

gsoldevila commented Mar 10, 2025 •

edited

Loading

Uh oh!

elasticmachine commented Mar 10, 2025

Uh oh!

Bamieh left a comment

Uh oh!

jloleysens left a comment

Uh oh!

jloleysens Mar 13, 2025

Uh oh!

jloleysens Mar 13, 2025

Uh oh!

jloleysens Mar 13, 2025

Uh oh!

jloleysens Mar 13, 2025

Uh oh!

elasticmachine commented Mar 13, 2025

Uh oh!

Uh oh!

gsoldevila commented Mar 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Comments

	/* We cannot unfreeze backing indices (that would break the related data_stream) */
	/* We cannot reindex backing indices in the same way as regular indices (that would break the related data_stream) */

	{/* We cannot unfreeze backing indices (that would break the related data_stream) */}
	{/* We cannot reindex backing indices in the same way as regular indices (that would break the related data_stream) */}

Conversation

gsoldevila commented Mar 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

old + open

old + closed

old + frozen + open

old + frozen + closed

old + open + manual write block (readonly)

old + closed + manual write block (readonly)

old + frozen + open + manual write block (readonly)

old + frozen + closed + manual write block (readonly)

Uh oh!

elasticmachine commented Mar 10, 2025

Uh oh!

Bamieh left a comment

Choose a reason for hiding this comment

Uh oh!

jloleysens left a comment

Choose a reason for hiding this comment

Uh oh!

jloleysens Mar 13, 2025

Choose a reason for hiding this comment

Uh oh!

jloleysens Mar 13, 2025

Choose a reason for hiding this comment

Uh oh!

jloleysens Mar 13, 2025

Choose a reason for hiding this comment

Uh oh!

jloleysens Mar 13, 2025

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Mar 13, 2025

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

History

Uh oh!

Uh oh!

gsoldevila commented Mar 13, 2025

💚 All backports created successfully

Questions ?

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Comments

gsoldevila commented Mar 10, 2025 •

edited

Loading