From 7e3d4fd11dc1e227c10db2e4a192b8725cad0876 Mon Sep 17 00:00:00 2001 From: eyalkoren <41850454+eyalkoren@users.noreply.github.com> Date: Tue, 25 Apr 2023 12:53:19 +0300 Subject: [PATCH 1/4] Fix issues with ignore_missing_component_templates --- .../metadata/ComposableIndexTemplate.java | 17 +++++++ .../MetadataIndexTemplateService.java | 16 ++++--- .../MetadataIndexTemplateServiceTests.java | 38 +++++++++++++++ .../org/elasticsearch/test/TestUtils.java | 26 ++++++++++ .../core/template/IndexTemplateRegistry.java | 2 +- ...StackRegistryWithNonRequiredTemplates.java | 37 ++++++++++++++ .../stack/StackTemplateRegistryTests.java | 48 +++++++++++++++++++ .../test/resources/non-required-template.json | 16 +++++++ 8 files changed, 193 insertions(+), 7 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/test/TestUtils.java create mode 100644 x-pack/plugin/stack/src/test/java/org/elasticsearch/xpack/stack/StackRegistryWithNonRequiredTemplates.java create mode 100644 x-pack/plugin/stack/src/test/resources/non-required-template.json diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplate.java b/server/src/main/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplate.java index ba8ecf96e7325..508f3235703fa 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplate.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/ComposableIndexTemplate.java @@ -198,6 +198,23 @@ public List composedOf() { return componentTemplates; } + /** + * Returns the required component templates, i.e. such that are not allowed to be missing, as in + * {@link #ignoreMissingComponentTemplates}. + * @return a list of required component templates + */ + public List getRequiredComponentTemplates() { + if (componentTemplates == null) { + return List.of(); + } + if (ignoreMissingComponentTemplates == null) { + return componentTemplates; + } + return componentTemplates.stream() + .filter(componentTemplate -> ignoreMissingComponentTemplates.contains(componentTemplate) == false) + .toList(); + } + @Nullable public Long priority() { return priority; diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java index 0aa8cc8685c90..e988bf13176bc 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java @@ -383,7 +383,7 @@ public void removeComponentTemplate( ClusterState state, final ActionListener listener ) { - validateNotInUse(state.metadata(), names); + validateCanBeRemoved(state.metadata(), names); taskQueue.submitTask("remove-component-template [" + String.join(",", names) + "]", new TemplateClusterStateUpdateTask(listener) { @Override public ClusterState execute(ClusterState currentState) { @@ -394,7 +394,7 @@ public ClusterState execute(ClusterState currentState) { // Exposed for ReservedComponentTemplateAction public static ClusterState innerRemoveComponentTemplate(ClusterState currentState, String... names) { - validateNotInUse(currentState.metadata(), names); + validateCanBeRemoved(currentState.metadata(), names); final Set templateNames = new HashSet<>(); if (names.length > 1) { @@ -439,10 +439,11 @@ public static ClusterState innerRemoveComponentTemplate(ClusterState currentStat } /** - * Validates that the given component template is not used by any index - * templates, throwing an error if it is still in use + * Validates that the given component template can be removed, throwing an error if it cannot. + * A component template should not be removed if it is required by any index templates, + * that is- it is used AND NOT specified as {@code ignore_missing_component_templates}. */ - static void validateNotInUse(Metadata metadata, String... templateNameOrWildcard) { + static void validateCanBeRemoved(Metadata metadata, String... templateNameOrWildcard) { final Predicate predicate; if (templateNameOrWildcard.length > 1) { predicate = name -> Arrays.asList(templateNameOrWildcard).contains(name); @@ -456,7 +457,10 @@ static void validateNotInUse(Metadata metadata, String... templateNameOrWildcard .collect(Collectors.toSet()); final Set componentsBeingUsed = new HashSet<>(); final List templatesStillUsing = metadata.templatesV2().entrySet().stream().filter(e -> { - Set intersecting = Sets.intersection(new HashSet<>(e.getValue().composedOf()), matchingComponentTemplates); + Set intersecting = Sets.intersection( + new HashSet<>(e.getValue().getRequiredComponentTemplates()), + matchingComponentTemplates + ); if (intersecting.size() > 0) { componentsBeingUsed.addAll(intersecting); return true; diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java index d05c2420cefb1..0b017d3e0143f 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java @@ -32,6 +32,7 @@ import org.elasticsearch.indices.InvalidIndexTemplateException; import org.elasticsearch.test.ClusterServiceUtils; import org.elasticsearch.test.ESSingleNodeTestCase; +import org.elasticsearch.test.TestUtils; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.xcontent.NamedXContentRegistry; import org.elasticsearch.xcontent.XContentParseException; @@ -46,6 +47,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.concurrent.Callable; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; @@ -1666,6 +1668,42 @@ public void testRemoveComponentTemplateInUse() throws Exception { ); } + public void testRemoveRequiredAndNonRequiredComponents() throws Exception { + ComposableIndexTemplate composableIndexTemplate = new ComposableIndexTemplate( + Collections.singletonList("pattern"), + null, + List.of("required1", "non-required", "required2"), + null, + null, + null, + null, + null, + Collections.singletonList("non-required") + ); + ComponentTemplate ct = new ComponentTemplate(new Template(null, new CompressedXContent("{}"), null), null, null); + + final MetadataIndexTemplateService service = getMetadataIndexTemplateService(); + ClusterState clusterState = service.addComponentTemplate(ClusterState.EMPTY_STATE, false, "required1", ct); + clusterState = service.addComponentTemplate(clusterState, false, "required2", ct); + clusterState = service.addComponentTemplate(clusterState, false, "non-required", ct); + clusterState = service.addIndexTemplateV2(clusterState, false, "composable-index-template", composableIndexTemplate); + + final ClusterState cs = clusterState; + Exception e = expectThrows(IllegalArgumentException.class, () -> innerRemoveComponentTemplate(cs, "required*")); + assertThat( + e.getMessage(), + containsString( + "component templates [required1, required2] cannot be removed as they are still in use by index templates " + + "[composable-index-template]" + ) + ); + + TestUtils.assetNoException((Callable) () -> { + innerRemoveComponentTemplate(cs, "non-required*"); + return null; + }, "component templates specified as 'ignore_missing_component_templates' can be removed"); + } + /** * Tests that we check that settings/mappings/etc are valid even after template composition, * when adding/updating a composable index template diff --git a/server/src/test/java/org/elasticsearch/test/TestUtils.java b/server/src/test/java/org/elasticsearch/test/TestUtils.java new file mode 100644 index 0000000000000..00ab5674f4df2 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/test/TestUtils.java @@ -0,0 +1,26 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.test; + +import org.elasticsearch.core.Nullable; + +import java.util.concurrent.Callable; + +/** + * Test utilities + */ +public class TestUtils { + public static T assetNoException(Callable callable, @Nullable String msg) { + try { + return callable.call(); + } catch (Throwable th) { + throw new AssertionError(((msg != null) ? msg + "; original error: " : "unexpected error: ") + th.getMessage()); + } + } +} diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/template/IndexTemplateRegistry.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/template/IndexTemplateRegistry.java index 255233f6ae461..5066b2d48f1c5 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/template/IndexTemplateRegistry.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/template/IndexTemplateRegistry.java @@ -327,7 +327,7 @@ private void addComposableTemplatesIfMissing(ClusterState state) { * Returns true if the cluster state contains all of the component templates needed by the composable template */ private static boolean componentTemplatesExist(ClusterState state, ComposableIndexTemplate indexTemplate) { - return state.metadata().componentTemplates().keySet().containsAll(indexTemplate.composedOf()); + return state.metadata().componentTemplates().keySet().containsAll(indexTemplate.getRequiredComponentTemplates()); } private void putLegacyTemplate(final IndexTemplateConfig config, final AtomicBoolean creationCheck) { diff --git a/x-pack/plugin/stack/src/test/java/org/elasticsearch/xpack/stack/StackRegistryWithNonRequiredTemplates.java b/x-pack/plugin/stack/src/test/java/org/elasticsearch/xpack/stack/StackRegistryWithNonRequiredTemplates.java new file mode 100644 index 0000000000000..7f674e24658dd --- /dev/null +++ b/x-pack/plugin/stack/src/test/java/org/elasticsearch/xpack/stack/StackRegistryWithNonRequiredTemplates.java @@ -0,0 +1,37 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.stack; + +import org.elasticsearch.client.internal.Client; +import org.elasticsearch.cluster.metadata.ComposableIndexTemplate; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.xcontent.NamedXContentRegistry; +import org.elasticsearch.xpack.core.template.IndexTemplateConfig; + +import java.util.Map; + +class StackRegistryWithNonRequiredTemplates extends StackTemplateRegistry { + StackRegistryWithNonRequiredTemplates( + Settings nodeSettings, + ClusterService clusterService, + ThreadPool threadPool, + Client client, + NamedXContentRegistry xContentRegistry + ) { + super(nodeSettings, clusterService, threadPool, client, xContentRegistry); + } + + @Override + protected Map getComposableTemplateConfigs() { + return parseComposableTemplates( + new IndexTemplateConfig("syslog", "/non-required-template.json", REGISTRY_VERSION, TEMPLATE_VERSION_VARIABLE) + ); + } +} diff --git a/x-pack/plugin/stack/src/test/java/org/elasticsearch/xpack/stack/StackTemplateRegistryTests.java b/x-pack/plugin/stack/src/test/java/org/elasticsearch/xpack/stack/StackTemplateRegistryTests.java index f3d41c3d96e44..6c23a752ac114 100644 --- a/x-pack/plugin/stack/src/test/java/org/elasticsearch/xpack/stack/StackTemplateRegistryTests.java +++ b/x-pack/plugin/stack/src/test/java/org/elasticsearch/xpack/stack/StackTemplateRegistryTests.java @@ -20,6 +20,7 @@ import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.block.ClusterBlocks; import org.elasticsearch.cluster.metadata.ComponentTemplate; +import org.elasticsearch.cluster.metadata.ComposableIndexTemplate; import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNodes; @@ -270,6 +271,53 @@ public void testThatUnversionedOldTemplatesAreUpgraded() throws Exception { assertBusy(() -> assertThat(calledTimes.get(), equalTo(registry.getComponentTemplateConfigs().size()))); } + public void testMissingNonRequiredTemplates() throws Exception { + StackRegistryWithNonRequiredTemplates registryWithNonRequiredTemplate = new StackRegistryWithNonRequiredTemplates( + Settings.EMPTY, + clusterService, + threadPool, + client, + NamedXContentRegistry.EMPTY + ); + + DiscoveryNode node = new DiscoveryNode("node", ESTestCase.buildNewFakeTransportAddress(), Version.CURRENT); + DiscoveryNodes nodes = DiscoveryNodes.builder().localNodeId("node").masterNodeId("node").add(node).build(); + + ClusterChangedEvent event = createClusterChangedEvent( + Collections.singletonMap(StackTemplateRegistry.LOGS_SETTINGS_COMPONENT_TEMPLATE_NAME, null), + nodes + ); + + final AtomicInteger calledTimes = new AtomicInteger(0); + client.setVerifier((action, request, listener) -> { + if (action instanceof PutComponentTemplateAction) { + // Ignore such + return AcknowledgedResponse.TRUE; + } else if (action instanceof PutLifecycleAction) { + // Ignore such + return AcknowledgedResponse.TRUE; + } else if (action instanceof PutComposableIndexTemplateAction) { + calledTimes.incrementAndGet(); + assertThat(request, instanceOf(PutComposableIndexTemplateAction.Request.class)); + PutComposableIndexTemplateAction.Request putComposableTemplateRequest = (PutComposableIndexTemplateAction.Request) request; + assertThat(putComposableTemplateRequest.name(), equalTo("syslog")); + ComposableIndexTemplate composableIndexTemplate = putComposableTemplateRequest.indexTemplate(); + assertThat(composableIndexTemplate.composedOf(), hasSize(2)); + assertThat(composableIndexTemplate.composedOf().get(0), equalTo("logs-settings")); + assertThat(composableIndexTemplate.composedOf().get(1), equalTo("syslog@custom")); + assertThat(composableIndexTemplate.getIgnoreMissingComponentTemplates(), hasSize(1)); + assertThat(composableIndexTemplate.getIgnoreMissingComponentTemplates().get(0), equalTo("syslog@custom")); + return AcknowledgedResponse.TRUE; + } else { + fail("client called with unexpected request:" + request.toString()); + return null; + } + }); + + registryWithNonRequiredTemplate.clusterChanged(event); + assertBusy(() -> assertThat(calledTimes.get(), equalTo(1))); + } + @TestLogging(value = "org.elasticsearch.xpack.core.template:DEBUG", reason = "test") public void testSameOrHigherVersionTemplateNotUpgraded() { DiscoveryNode node = new DiscoveryNode("node", ESTestCase.buildNewFakeTransportAddress(), Version.CURRENT); diff --git a/x-pack/plugin/stack/src/test/resources/non-required-template.json b/x-pack/plugin/stack/src/test/resources/non-required-template.json new file mode 100644 index 0000000000000..3011d0171399d --- /dev/null +++ b/x-pack/plugin/stack/src/test/resources/non-required-template.json @@ -0,0 +1,16 @@ +{ + "index_patterns": ["syslog-*-*"], + "priority": 100, + "data_stream": {}, + "composed_of": [ + "logs-settings", + "syslog@custom" + ], + "ignore_missing_component_templates": ["syslog@custom"], + "allow_auto_create": true, + "_meta": { + "description": "default logs template installed by x-pack", + "managed": true + }, + "version": ${xpack.stack.template.version} +} From 4a676c97640f7807aa8f9148ef478b6964a916a9 Mon Sep 17 00:00:00 2001 From: eyalkoren <41850454+eyalkoren@users.noreply.github.com> Date: Tue, 25 Apr 2023 13:17:17 +0300 Subject: [PATCH 2/4] Update docs/changelog/95527.yaml --- docs/changelog/95527.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/95527.yaml diff --git a/docs/changelog/95527.yaml b/docs/changelog/95527.yaml new file mode 100644 index 0000000000000..b5dd70bf74303 --- /dev/null +++ b/docs/changelog/95527.yaml @@ -0,0 +1,5 @@ +pr: 95527 +summary: Fix issues with `ignore_missing_component_templates` +area: Data streams +type: bug +issues: [] From fcec158c62ee717aa886d74aa5810d96e070e383 Mon Sep 17 00:00:00 2001 From: eyalkoren <41850454+eyalkoren@users.noreply.github.com> Date: Thu, 27 Apr 2023 08:21:33 +0300 Subject: [PATCH 3/4] Removing test utility --- .../MetadataIndexTemplateServiceTests.java | 8 ++---- .../org/elasticsearch/test/TestUtils.java | 26 ------------------- 2 files changed, 2 insertions(+), 32 deletions(-) delete mode 100644 server/src/test/java/org/elasticsearch/test/TestUtils.java diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java index 0b017d3e0143f..61cc9ce1f0ba4 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateServiceTests.java @@ -32,7 +32,6 @@ import org.elasticsearch.indices.InvalidIndexTemplateException; import org.elasticsearch.test.ClusterServiceUtils; import org.elasticsearch.test.ESSingleNodeTestCase; -import org.elasticsearch.test.TestUtils; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.xcontent.NamedXContentRegistry; import org.elasticsearch.xcontent.XContentParseException; @@ -47,7 +46,6 @@ import java.util.Map; import java.util.Objects; import java.util.Set; -import java.util.concurrent.Callable; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; @@ -1698,10 +1696,8 @@ public void testRemoveRequiredAndNonRequiredComponents() throws Exception { ) ); - TestUtils.assetNoException((Callable) () -> { - innerRemoveComponentTemplate(cs, "non-required*"); - return null; - }, "component templates specified as 'ignore_missing_component_templates' can be removed"); + // This removal should succeed + innerRemoveComponentTemplate(cs, "non-required*"); } /** diff --git a/server/src/test/java/org/elasticsearch/test/TestUtils.java b/server/src/test/java/org/elasticsearch/test/TestUtils.java deleted file mode 100644 index 00ab5674f4df2..0000000000000 --- a/server/src/test/java/org/elasticsearch/test/TestUtils.java +++ /dev/null @@ -1,26 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -package org.elasticsearch.test; - -import org.elasticsearch.core.Nullable; - -import java.util.concurrent.Callable; - -/** - * Test utilities - */ -public class TestUtils { - public static T assetNoException(Callable callable, @Nullable String msg) { - try { - return callable.call(); - } catch (Throwable th) { - throw new AssertionError(((msg != null) ? msg + "; original error: " : "unexpected error: ") + th.getMessage()); - } - } -} From f718bbf44070eccaa8f2f9c2049ad14cc3edbad2 Mon Sep 17 00:00:00 2001 From: eyalkoren <41850454+eyalkoren@users.noreply.github.com> Date: Thu, 27 Apr 2023 08:46:58 +0300 Subject: [PATCH 4/4] Modify changelog description --- docs/changelog/95527.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog/95527.yaml b/docs/changelog/95527.yaml index b5dd70bf74303..2c85521361644 100644 --- a/docs/changelog/95527.yaml +++ b/docs/changelog/95527.yaml @@ -1,5 +1,5 @@ pr: 95527 -summary: Fix issues with `ignore_missing_component_templates` +summary: Allow deletion of component templates that are specified in the `ignore_missing_component_templates` array area: Data streams type: bug issues: []