Skip to content

Commit 8d61b4f

Browse files
grcevskigrcevski
authored
Backport of #87933 (#87979)
1 parent 6c0ccdf commit 8d61b4f

File tree

9 files changed

+258
-6
lines changed

9 files changed

+258
-6
lines changed

‎docs/changelog/87933.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 87933
2+
summary: Prevent migration of indices that match templates
3+
area: Infra/Core
4+
type: bug
5+
issues: [86801, 87827]

‎modules/kibana/src/main/java/org/elasticsearch/kibana/KibanaPlugin.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ public class KibanaPlugin extends Plugin implements SystemIndexPlugin {
3131
.setAliasName(".kibana")
3232
.setType(Type.EXTERNAL_UNMANAGED)
3333
.setAllowedElasticProductOrigins(KIBANA_PRODUCT_ORIGIN)
34+
.setAllowsTemplates()
3435
.build();
3536

3637
public static final SystemIndexDescriptor REPORTING_INDEX_DESCRIPTOR = SystemIndexDescriptor.builder()

‎modules/reindex/src/internalClusterTest/java/org/elasticsearch/migration/AbstractFeatureMigrationIntegTest.java‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,18 @@ public abstract class AbstractFeatureMigrationIntegTest extends ESIntegTestCase
119119
static final int EXTERNAL_UNMANAGED_FLAG_VALUE = 4;
120120
static final String ASSOCIATED_INDEX_NAME = ".my-associated-idx";
121121

122+
public static final SystemIndexDescriptor KIBANA_MOCK_INDEX_DESCRIPTOR = SystemIndexDescriptor.builder()
123+
.setIndexPattern(".kibana_*")
124+
.setDescription("Kibana saved objects system index")
125+
.setAliasName(".kibana")
126+
.setType(SystemIndexDescriptor.Type.EXTERNAL_UNMANAGED)
127+
.setAllowedElasticProductOrigins(Collections.emptyList())
128+
.setAllowedElasticProductOrigins(Collections.singletonList(ORIGIN))
129+
.setMinimumNodeVersion(NEEDS_UPGRADE_VERSION)
130+
.setPriorSystemIndexDescriptors(Collections.emptyList())
131+
.setAllowsTemplates()
132+
.build();
133+
122134
protected String masterAndDataNode;
123135
protected String masterName;
124136

@@ -271,7 +283,7 @@ public String getFeatureDescription() {
271283

272284
@Override
273285
public Collection<SystemIndexDescriptor> getSystemIndexDescriptors(Settings settings) {
274-
return Arrays.asList(INTERNAL_MANAGED, INTERNAL_UNMANAGED, EXTERNAL_MANAGED, EXTERNAL_UNMANAGED);
286+
return Arrays.asList(INTERNAL_MANAGED, INTERNAL_UNMANAGED, EXTERNAL_MANAGED, EXTERNAL_UNMANAGED, KIBANA_MOCK_INDEX_DESCRIPTOR);
275287
}
276288

277289
@Override

‎modules/reindex/src/internalClusterTest/java/org/elasticsearch/migration/FeatureMigrationIT.java‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,24 @@
1515
import org.elasticsearch.action.admin.cluster.migration.PostFeatureUpgradeAction;
1616
import org.elasticsearch.action.admin.cluster.migration.PostFeatureUpgradeRequest;
1717
import org.elasticsearch.action.admin.cluster.migration.PostFeatureUpgradeResponse;
18+
import org.elasticsearch.action.admin.indices.alias.Alias;
1819
import org.elasticsearch.action.admin.indices.create.CreateIndexRequestBuilder;
1920
import org.elasticsearch.action.admin.indices.create.CreateIndexResponse;
21+
import org.elasticsearch.action.admin.indices.template.put.PutComponentTemplateAction;
22+
import org.elasticsearch.action.admin.indices.template.put.PutComposableIndexTemplateAction;
2023
import org.elasticsearch.action.support.ActiveShardCount;
2124
import org.elasticsearch.cluster.ClusterState;
2225
import org.elasticsearch.cluster.ClusterStateUpdateTask;
26+
import org.elasticsearch.cluster.metadata.ComponentTemplate;
27+
import org.elasticsearch.cluster.metadata.ComposableIndexTemplate;
2328
import org.elasticsearch.cluster.metadata.IndexMetadata;
2429
import org.elasticsearch.cluster.metadata.Metadata;
30+
import org.elasticsearch.cluster.metadata.Template;
2531
import org.elasticsearch.cluster.service.ClusterService;
2632
import org.elasticsearch.common.Strings;
33+
import org.elasticsearch.common.compress.CompressedXContent;
2734
import org.elasticsearch.common.settings.Settings;
35+
import org.elasticsearch.indices.SystemIndexDescriptor;
2836
import org.elasticsearch.plugins.Plugin;
2937
import org.elasticsearch.reindex.ReindexPlugin;
3038
import org.elasticsearch.upgrades.FeatureMigrationResults;
@@ -310,4 +318,145 @@ public void onFailure(String source, Exception e) {
310318
assertThat(statusResp.getUpgradeStatus(), equalTo(GetFeatureUpgradeStatusResponse.UpgradeStatus.NO_MIGRATION_NEEDED));
311319
});
312320
}
321+
322+
private String featureUpgradeErrorResponse(GetFeatureUpgradeStatusResponse statusResp) {
323+
return statusResp.getFeatureUpgradeStatuses()
324+
.stream()
325+
.map(f -> f.getIndexVersions())
326+
.flatMap(List::stream)
327+
.map(i -> (i.getException() == null) ? "" : i.getException().getMessage())
328+
.collect(Collectors.joining(" "));
329+
}
330+
331+
private void migrateWithTemplatesV1(String templatePrefix, SystemIndexDescriptor... descriptors) throws Exception {
332+
for (SystemIndexDescriptor descriptor : descriptors) {
333+
createSystemIndexForDescriptor(descriptor);
334+
}
335+
336+
client().admin()
337+
.indices()
338+
.preparePutTemplate("bad_template")
339+
.setPatterns(Collections.singletonList(templatePrefix + "*"))
340+
.addAlias(new Alias(templatePrefix + "-legacy-alias"))
341+
.get();
342+
343+
ensureGreen();
344+
345+
PostFeatureUpgradeResponse migrationResponse = client().execute(PostFeatureUpgradeAction.INSTANCE, new PostFeatureUpgradeRequest())
346+
.get();
347+
348+
assertTrue(migrationResponse.isAccepted());
349+
}
350+
351+
public void testBailOnMigrateWithTemplatesV1() throws Exception {
352+
migrateWithTemplatesV1(".int", INTERNAL_UNMANAGED);
353+
354+
assertBusy(() -> {
355+
GetFeatureUpgradeStatusResponse statusResp = client().execute(
356+
GetFeatureUpgradeStatusAction.INSTANCE,
357+
new GetFeatureUpgradeStatusRequest()
358+
).get();
359+
logger.info(Strings.toString(statusResp));
360+
assertThat(statusResp.getUpgradeStatus(), equalTo(GetFeatureUpgradeStatusResponse.UpgradeStatus.ERROR));
361+
assertTrue(featureUpgradeErrorResponse(statusResp).contains(" because it would match legacy templates "));
362+
});
363+
}
364+
365+
public void testMigrateWithTemplatesV1() throws Exception {
366+
// this should pass for both, kibana allows templates, the unmanaged doesn't match the template
367+
migrateWithTemplatesV1(".kibana", KIBANA_MOCK_INDEX_DESCRIPTOR, INTERNAL_UNMANAGED);
368+
369+
assertBusy(() -> {
370+
GetFeatureUpgradeStatusResponse statusResp = client().execute(
371+
GetFeatureUpgradeStatusAction.INSTANCE,
372+
new GetFeatureUpgradeStatusRequest()
373+
).get();
374+
logger.info(Strings.toString(statusResp));
375+
assertThat(statusResp.getUpgradeStatus(), equalTo(GetFeatureUpgradeStatusResponse.UpgradeStatus.NO_MIGRATION_NEEDED));
376+
});
377+
}
378+
379+
private void migrateWithTemplatesV2(String prefix, SystemIndexDescriptor... descriptors) throws Exception {
380+
for (SystemIndexDescriptor descriptor : descriptors) {
381+
createSystemIndexForDescriptor(descriptor);
382+
}
383+
384+
ComponentTemplate ct = new ComponentTemplate(
385+
new Template(
386+
null,
387+
new CompressedXContent(
388+
"{\n"
389+
+ " \"dynamic\": false,\n"
390+
+ " \"properties\": {\n"
391+
+ " \"field1\": {\n"
392+
+ " \"type\": \"text\"\n"
393+
+ " }\n"
394+
+ " }\n"
395+
+ " }"
396+
),
397+
null
398+
),
399+
3L,
400+
Collections.singletonMap("foo", "bar")
401+
);
402+
client().execute(PutComponentTemplateAction.INSTANCE, new PutComponentTemplateAction.Request("a-ct").componentTemplate(ct)).get();
403+
404+
ComposableIndexTemplate cit = new ComposableIndexTemplate(
405+
Collections.singletonList(prefix + "*"),
406+
new Template(
407+
null,
408+
new CompressedXContent(
409+
"{\n"
410+
+ " \"dynamic\": false,\n"
411+
+ " \"properties\": {\n"
412+
+ " \"field2\": {\n"
413+
+ " \"type\": \"keyword\"\n"
414+
+ " }\n"
415+
+ " }\n"
416+
+ " }"
417+
),
418+
null
419+
),
420+
Collections.singletonList("a-ct"),
421+
4L,
422+
5L,
423+
Collections.singletonMap("baz", "thud")
424+
);
425+
client().execute(PutComposableIndexTemplateAction.INSTANCE, new PutComposableIndexTemplateAction.Request("a-it").indexTemplate(cit))
426+
.get();
427+
428+
ensureGreen();
429+
430+
PostFeatureUpgradeResponse migrationResponse = client().execute(PostFeatureUpgradeAction.INSTANCE, new PostFeatureUpgradeRequest())
431+
.get();
432+
assertTrue(migrationResponse.isAccepted());
433+
}
434+
435+
public void testBailOnMigrateWithTemplatesV2() throws Exception {
436+
migrateWithTemplatesV2(".int", INTERNAL_UNMANAGED);
437+
438+
assertBusy(() -> {
439+
GetFeatureUpgradeStatusResponse statusResp = client().execute(
440+
GetFeatureUpgradeStatusAction.INSTANCE,
441+
new GetFeatureUpgradeStatusRequest()
442+
).get();
443+
logger.info(Strings.toString(statusResp));
444+
assertThat(statusResp.getUpgradeStatus(), equalTo(GetFeatureUpgradeStatusResponse.UpgradeStatus.ERROR));
445+
assertTrue(featureUpgradeErrorResponse(statusResp).contains(" it would match composable template [a-it]"));
446+
});
447+
}
448+
449+
public void testMigrateWithTemplatesV2() throws Exception {
450+
// this should pass for both, kibana allows templates, the unmanaged doesn't match the template
451+
migrateWithTemplatesV2(".kibana", KIBANA_MOCK_INDEX_DESCRIPTOR, INTERNAL_UNMANAGED);
452+
453+
assertBusy(() -> {
454+
GetFeatureUpgradeStatusResponse statusResp = client().execute(
455+
GetFeatureUpgradeStatusAction.INSTANCE,
456+
new GetFeatureUpgradeStatusRequest()
457+
).get();
458+
logger.info(Strings.toString(statusResp));
459+
assertThat(statusResp.getUpgradeStatus(), equalTo(GetFeatureUpgradeStatusResponse.UpgradeStatus.NO_MIGRATION_NEEDED));
460+
});
461+
}
313462
}

‎server/src/internalClusterTest/java/org/elasticsearch/indices/TestSystemIndexDescriptor.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ public class TestSystemIndexDescriptor extends SystemIndexDescriptor {
5656
emptyList(),
5757
emptyList(),
5858
null,
59+
false,
5960
false
6061
);
6162
}

‎server/src/main/java/org/elasticsearch/cluster/metadata/IndexTemplateMetadata.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,11 @@ public int hashCode() {
189189
return result;
190190
}
191191

192+
@Override
193+
public String toString() {
194+
return name;
195+
}
196+
192197
public static IndexTemplateMetadata readFrom(StreamInput in) throws IOException {
193198
Builder builder = new Builder(in.readString());
194199
builder.order(in.readInt());

‎server/src/main/java/org/elasticsearch/indices/SystemIndexDescriptor.java‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,13 @@ public class SystemIndexDescriptor implements IndexPatternMatcher, Comparable<Sy
112112

113113
private final boolean isNetNew;
114114

115+
/**
116+
* We typically don't want to apply user defined templates on system indices, since they may have unexpected
117+
* behaviour when upgrading Elasticsearch versions. Currently, only the .kibana_ indices use templates, so we
118+
* are making this property by default as false.
119+
*/
120+
private final boolean allowsTemplates;
121+
115122
/**
116123
* The thread pools that actions will use to operate on this descriptor's system indices
117124
*/
@@ -142,6 +149,7 @@ public SystemIndexDescriptor(String indexPattern, String description) {
142149
emptyList(),
143150
emptyList(),
144151
null,
152+
false,
145153
false
146154
);
147155
}
@@ -174,6 +182,7 @@ public SystemIndexDescriptor(String indexPattern, String description, Type type,
174182
allowedElasticProductOrigins,
175183
emptyList(),
176184
null,
185+
false,
177186
false
178187
);
179188
}
@@ -201,6 +210,7 @@ public SystemIndexDescriptor(String indexPattern, String description, Type type,
201210
* indices
202211
* @param priorSystemIndexDescriptors A list of system index descriptors that describe the same index in a way that is compatible with
203212
* older versions of Elasticsearch
213+
* @param allowsTemplates if this system index descriptor allows templates to affect its settings (e.g. .kibana_ indices)
204214
*/
205215
SystemIndexDescriptor(
206216
String indexPattern,
@@ -218,7 +228,8 @@ public SystemIndexDescriptor(String indexPattern, String description, Type type,
218228
List<String> allowedElasticProductOrigins,
219229
List<SystemIndexDescriptor> priorSystemIndexDescriptors,
220230
ExecutorNames executorNames,
221-
boolean isNetNew
231+
boolean isNetNew,
232+
boolean allowsTemplates
222233
) {
223234
Objects.requireNonNull(indexPattern, "system index pattern must not be null");
224235
if (indexPattern.length() < 2) {
@@ -369,6 +380,7 @@ public SystemIndexDescriptor(String indexPattern, String description, Type type,
369380
this.priorSystemIndexDescriptors = sortedPriorSystemIndexDescriptors;
370381
this.executorNames = Objects.nonNull(executorNames) ? executorNames : ExecutorNames.DEFAULT_SYSTEM_INDEX_THREAD_POOLS;
371382
this.isNetNew = isNetNew;
383+
this.allowsTemplates = allowsTemplates;
372384
}
373385

374386
/**
@@ -484,6 +496,10 @@ public boolean isNetNew() {
484496
return isNetNew;
485497
}
486498

499+
public boolean allowsTemplates() {
500+
return allowsTemplates;
501+
}
502+
487503
public Version getMappingVersion() {
488504
if (type.isManaged() == false) {
489505
throw new IllegalStateException(this + " is not managed so there are no mappings or version");
@@ -610,6 +626,7 @@ public static class Builder {
610626
private List<SystemIndexDescriptor> priorSystemIndexDescriptors = emptyList();
611627
private ExecutorNames executorNames;
612628
private boolean isNetNew = false;
629+
private boolean allowsTemplates = false;
613630

614631
private Builder() {}
615632

@@ -703,12 +720,16 @@ public Builder setNetNew() {
703720
return this;
704721
}
705722

723+
public Builder setAllowsTemplates() {
724+
this.allowsTemplates = true;
725+
return this;
726+
}
727+
706728
/**
707729
* Builds a {@link SystemIndexDescriptor} using the fields supplied to this builder.
708730
* @return a populated descriptor.
709731
*/
710732
public SystemIndexDescriptor build() {
711-
712733
return new SystemIndexDescriptor(
713734
indexPattern,
714735
primaryIndex,
@@ -725,7 +746,8 @@ public SystemIndexDescriptor build() {
725746
allowedElasticProductOrigins,
726747
priorSystemIndexDescriptors,
727748
executorNames,
728-
isNetNew
749+
isNetNew,
750+
allowsTemplates
729751
);
730752
}
731753
}

‎server/src/main/java/org/elasticsearch/upgrades/SystemIndexMigrationInfo.java‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ class SystemIndexMigrationInfo implements Comparable<SystemIndexMigrationInfo> {
4949
private final String origin;
5050
private final String indexType;
5151
private final SystemIndices.Feature owningFeature;
52+
private final boolean allowsTemplates;
5253

5354
private static final Comparator<SystemIndexMigrationInfo> SAME_CLASS_COMPARATOR = Comparator.comparing(
5455
SystemIndexMigrationInfo::getFeatureName
@@ -61,7 +62,8 @@ private SystemIndexMigrationInfo(
6162
String mapping,
6263
String origin,
6364
String indexType,
64-
SystemIndices.Feature owningFeature
65+
SystemIndices.Feature owningFeature,
66+
boolean allowsTemplates
6567
) {
6668
this.currentIndex = currentIndex;
6769
this.featureName = featureName;
@@ -70,6 +72,7 @@ private SystemIndexMigrationInfo(
7072
this.origin = origin;
7173
this.indexType = indexType;
7274
this.owningFeature = owningFeature;
75+
this.allowsTemplates = allowsTemplates;
7376
}
7477

7578
/**
@@ -128,6 +131,16 @@ String getIndexType() {
128131
return indexType;
129132
}
130133

134+
/**
135+
* By default, system indices should not be affected by user defined templates, so this
136+
* method should return false in almost all cases. At the moment certain Kibana indices use
137+
* templates, therefore we allow templates to be used on Kibana created system indices until
138+
* Kibana removes the template use on system index creation.
139+
*/
140+
boolean allowsTemplates() {
141+
return allowsTemplates;
142+
}
143+
131144
/**
132145
* Invokes the pre-migration hook for the feature that owns this index.
133146
* See {@link SystemIndexPlugin#prepareForIndicesMigration(ClusterService, Client, ActionListener)}.
@@ -237,7 +250,8 @@ static SystemIndexMigrationInfo build(
237250
mapping,
238251
descriptor.getOrigin(),
239252
descriptor.getIndexType(),
240-
feature
253+
feature,
254+
descriptor.allowsTemplates()
241255
);
242256
}
243257

0 commit comments

Comments
 (0)