Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

System indices ignore all user templates #87260

Merged
merged 7 commits into from
Jun 30, 2022
Merged

System indices ignore all user templates #87260

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8444d88
Ignore templates for system indices
williamrandolph May 26, 2022
f95bd79
Update docs/changelog/87260.yaml
williamrandolph May 31, 2022
b319de0
Remove unneeded test
williamrandolph May 31, 2022
fff1130
Merge branch 'master' into si2/system-indices-ignore-templates
Jun 23, 2022
996e7cb
Check for allowsTemplates and add tests
Jun 23, 2022
aa85a9c
Apply review suggestions
Jun 28, 2022
d859145
Merge branch 'master' into si2/system-indices-ignore-templates
elasticmachine Jun 29, 2022
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: 5 additions & 0 deletions docs/changelog/87260.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 87260
summary: System indices ignore all user templates
area: Infra/Core
type: bug
issues: []
Copy link
Contributor

Choose a reason for hiding this comment

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

Remember to add the relevant issue links here.

44 changes: 26 additions & 18 deletions qa/system-indices/src/javaRestTest/java/org/elasticsearch/system/indices/SystemAliasIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ public void testCreatingSystemIndexWithAlias() throws Exception {
assertThat(response.getStatusLine().getStatusCode(), is(200));
}

assertAliasIsHiddenInIndexResponse(".internal-unmanaged-index-8", ".internal-unmanaged-alias");
assertAliasIsHiddenInAliasesEndpoint(".internal-unmanaged-index-8", ".internal-unmanaged-alias");
assertAliasIsHiddenInIndexResponse(".internal-unmanaged-index-8", ".internal-unmanaged-alias", true);
assertAliasIsHiddenInAliasesEndpoint(".internal-unmanaged-index-8", ".internal-unmanaged-alias", true);
}

public void testCreatingSystemIndexWithLegacyAlias() throws Exception {
Expand All @@ -71,8 +71,8 @@ public void testCreatingSystemIndexWithLegacyAlias() throws Exception {
assertThat(response.getStatusLine().getStatusCode(), is(200));
}

assertAliasIsHiddenInIndexResponse(".internal-unmanaged-index-8", ".internal-unmanaged-alias");
assertAliasIsHiddenInAliasesEndpoint(".internal-unmanaged-index-8", ".internal-unmanaged-alias");
assertAliasIsHiddenInIndexResponse(".internal-unmanaged-index-8", ".internal-unmanaged-alias", false);
assertAliasIsHiddenInAliasesEndpoint(".internal-unmanaged-index-8", ".internal-unmanaged-alias", false);
}

public void testCreatingSystemIndexWithIndexAliasEndpoint() throws Exception {
Expand All @@ -94,8 +94,8 @@ public void testCreatingSystemIndexWithIndexAliasEndpoint() throws Exception {
assertThat(response.getStatusLine().getStatusCode(), is(200));
}

assertAliasIsHiddenInIndexResponse(".internal-unmanaged-index-8", ".internal-unmanaged-alias");
assertAliasIsHiddenInAliasesEndpoint(".internal-unmanaged-index-8", ".internal-unmanaged-alias");
assertAliasIsHiddenInIndexResponse(".internal-unmanaged-index-8", ".internal-unmanaged-alias", true);
assertAliasIsHiddenInAliasesEndpoint(".internal-unmanaged-index-8", ".internal-unmanaged-alias", true);
}

public void testCreatingSystemIndexWithAliasEndpoint() throws Exception {
Expand All @@ -118,8 +118,8 @@ public void testCreatingSystemIndexWithAliasEndpoint() throws Exception {
assertThat(response.getStatusLine().getStatusCode(), is(200));
}

assertAliasIsHiddenInIndexResponse(".internal-unmanaged-index-8", ".internal-unmanaged-alias");
assertAliasIsHiddenInAliasesEndpoint(".internal-unmanaged-index-8", ".internal-unmanaged-alias");
assertAliasIsHiddenInIndexResponse(".internal-unmanaged-index-8", ".internal-unmanaged-alias", true);
assertAliasIsHiddenInAliasesEndpoint(".internal-unmanaged-index-8", ".internal-unmanaged-alias", true);
}

public void testCreatingSystemIndexWithAliasesEndpoint() throws Exception {
Expand Down Expand Up @@ -154,12 +154,12 @@ public void testCreatingSystemIndexWithAliasesEndpoint() throws Exception {
assertThat(response.getStatusLine().getStatusCode(), is(200));
}

assertAliasIsHiddenInIndexResponse(".internal-unmanaged-index-8", ".internal-unmanaged-alias");
assertAliasIsHiddenInAliasesEndpoint(".internal-unmanaged-index-8", ".internal-unmanaged-alias");
assertAliasIsHiddenInIndexResponse(".internal-unmanaged-index-8", ".internal-unmanaged-alias", true);
assertAliasIsHiddenInAliasesEndpoint(".internal-unmanaged-index-8", ".internal-unmanaged-alias", true);
}

@SuppressWarnings("unchecked")
private void assertAliasIsHiddenInIndexResponse(String indexName, String aliasName) throws IOException {
private void assertAliasIsHiddenInIndexResponse(String indexName, String aliasName, boolean expectMatch) throws IOException {
Request request = new Request("GET", "/" + indexName);
request.setOptions(
expectWarnings(
Expand All @@ -177,14 +177,18 @@ private void assertAliasIsHiddenInIndexResponse(String indexName, String aliasNa
assertThat(indexSettingsMap.get("hidden"), equalTo("true"));

Map<String, Object> aliasesMap = (Map<String, Object>) indexMap.get("aliases");
assertThat(aliasesMap.keySet(), equalTo(Set.of(aliasName)));
Map<String, Object> aliasMap = (Map<String, Object>) aliasesMap.get(aliasName);
assertThat(aliasMap.get("is_hidden"), notNullValue());
assertThat(aliasMap.get("is_hidden"), equalTo(true));
if (expectMatch == false) {
assertTrue(aliasesMap.keySet().isEmpty());
} else {
assertThat(aliasesMap.keySet(), equalTo(Set.of(aliasName)));
Map<String, Object> aliasMap = (Map<String, Object>) aliasesMap.get(aliasName);
assertThat(aliasMap.get("is_hidden"), notNullValue());
assertThat(aliasMap.get("is_hidden"), equalTo(true));
}
}

@SuppressWarnings("unchecked")
private void assertAliasIsHiddenInAliasesEndpoint(String indexName, String aliasName) throws IOException {
private void assertAliasIsHiddenInAliasesEndpoint(String indexName, String aliasName, boolean expectMatch) throws IOException {
Request request = new Request("GET", "/_aliases");
request.setOptions(
expectWarnings(
Expand All @@ -199,7 +203,11 @@ private void assertAliasIsHiddenInAliasesEndpoint(String indexName, String alias
Map<String, Object> indexAliasMap = (Map<String, Object>) responseMap.get(indexName);
Map<String, Object> aliasesMap = (Map<String, Object>) indexAliasMap.get("aliases");
Map<String, Object> aliasMap = (Map<String, Object>) aliasesMap.get(aliasName);
assertThat(aliasMap.get("is_hidden"), notNullValue());
assertThat(aliasMap.get("is_hidden"), equalTo(true));
if (expectMatch == false) {
assertNull(aliasMap);
} else {
assertThat(aliasMap.get("is_hidden"), notNullValue());
assertThat(aliasMap.get("is_hidden"), equalTo(true));
}
}
}
88 changes: 71 additions & 17 deletions ...usterTest/java/org/elasticsearch/action/admin/indices/create/AutoCreateSystemIndexIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.CollectionUtils;
import org.elasticsearch.indices.SystemIndexDescriptor;
import org.elasticsearch.indices.TestSystemIndexDescriptorAllowsTemplates;
import org.elasticsearch.indices.TestSystemIndexPlugin;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.plugins.SystemIndexPlugin;
Expand Down Expand Up @@ -167,40 +168,60 @@ public void testSystemIndicesAutoCreateRejectedWhenNotHidden() {
);
}

/**
* Check that a template applying a system alias creates a hidden alias.
*/
public void testAutoCreateSystemAliasViaV1Template() throws Exception {
private String autoCreateSystemAliasViaV1Template(String indexName) throws Exception {
assertAcked(
client().admin()
.indices()
.preparePutTemplate("test-template")
.setPatterns(List.of(INDEX_NAME + "*"))
.addAlias(new Alias(INDEX_NAME + "-legacy-alias"))
.setPatterns(List.of(indexName + "*"))
.addAlias(new Alias(indexName + "-legacy-alias"))
.get()
);

String nonPrimaryIndex = INDEX_NAME + "-2";
String nonPrimaryIndex = indexName + "-2";
CreateIndexRequest request = new CreateIndexRequest(nonPrimaryIndex);
assertAcked(client().execute(AutoCreateAction.INSTANCE, request).get());

assertTrue(indexExists(nonPrimaryIndex));

assertAliasesHidden(nonPrimaryIndex, Set.of(".test-index", ".test-index-legacy-alias"));
return nonPrimaryIndex;
}

/**
* Check that a legacy template does not create an alias for a system index
*/
public void testAutoCreateSystemAliasViaV1Template() throws Exception {
var nonPrimaryIndex = autoCreateSystemAliasViaV1Template(INDEX_NAME);

assertAliasesHidden(nonPrimaryIndex, Set.of(INDEX_NAME), 1);

assertAcked(client().admin().indices().prepareDeleteTemplate("*").get());
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, it looks like this line was copied - can you remove it from here as well? It's equally unnecessary.

}

/**
* Check that a composable template applying a system alias creates a hidden alias.
* Check that a legacy template does create an alias for a system index, because of allows templates
*/
public void testAutoCreateSystemAliasViaComposableTemplate() throws Exception {
public void testAutoCreateSystemAliasViaV1TemplateAllowsTemplates() throws Exception {
var nonPrimaryIndex = autoCreateSystemAliasViaV1Template(TestSystemIndexDescriptorAllowsTemplates.INDEX_NAME);

assertAliasesHidden(
nonPrimaryIndex,
Set.of(
TestSystemIndexDescriptorAllowsTemplates.INDEX_NAME,
TestSystemIndexDescriptorAllowsTemplates.INDEX_NAME + "-legacy-alias"
),
2
);

assertAcked(client().admin().indices().prepareDeleteTemplate("*").get());
Copy link
Contributor

Choose a reason for hiding this comment

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

This line isn't necessary, it's done in the @After (and this should be handled by the test infrastructure anyway, I thought).

}

private String autoCreateSystemAliasViaComposableTemplate(String indexName) throws Exception {
ComposableIndexTemplate cit = new ComposableIndexTemplate(
Collections.singletonList(INDEX_NAME + "*"),
Collections.singletonList(indexName + "*"),
new Template(
null,
null,
Map.of(INDEX_NAME + "-composable-alias", AliasMetadata.builder(INDEX_NAME + "-composable-alias").build())
Map.of(indexName + "-composable-alias", AliasMetadata.builder(indexName + "-composable-alias").build())
),
Collections.emptyList(),
4L,
Expand All @@ -214,13 +235,45 @@ public void testAutoCreateSystemAliasViaComposableTemplate() throws Exception {
).get()
);

String nonPrimaryIndex = INDEX_NAME + "-2";
String nonPrimaryIndex = indexName + "-2";
CreateIndexRequest request = new CreateIndexRequest(nonPrimaryIndex);
assertAcked(client().execute(AutoCreateAction.INSTANCE, request).get());

assertTrue(indexExists(nonPrimaryIndex));

assertAliasesHidden(nonPrimaryIndex, Set.of(".test-index", ".test-index-composable-alias"));
return nonPrimaryIndex;
}

/**
* Check that a composable template does not create an alias for a system index
*/
public void testAutoCreateSystemAliasViaComposableTemplate() throws Exception {
String nonPrimaryIndex = autoCreateSystemAliasViaComposableTemplate(INDEX_NAME);

assertAliasesHidden(nonPrimaryIndex, Set.of(INDEX_NAME), 1);

assertAcked(
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm pretty sure this is unnecessary as well despite not being explicitly in the @After, I ran it locally without this x10 and every thing, passed, and :server:check passed as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm inclined to leave it, it does clean up the template explicitly and at the same time it tests we can clean up composable templates.

client().execute(
DeleteComposableIndexTemplateAction.INSTANCE,
new DeleteComposableIndexTemplateAction.Request("test-composable-template")
).get()
);
}

/**
* Check that a composable template does create an alias for a system index, because of allows templates
*/
public void testAutoCreateSystemAliasViaComposableTemplateAllowsTemplates() throws Exception {
String nonPrimaryIndex = autoCreateSystemAliasViaComposableTemplate(TestSystemIndexDescriptorAllowsTemplates.INDEX_NAME);

assertAliasesHidden(
nonPrimaryIndex,
Set.of(
TestSystemIndexDescriptorAllowsTemplates.INDEX_NAME,
TestSystemIndexDescriptorAllowsTemplates.INDEX_NAME + "-composable-alias"
),
2
);

assertAcked(
client().execute(
Expand All @@ -230,14 +283,15 @@ public void testAutoCreateSystemAliasViaComposableTemplate() throws Exception {
);
}

private void assertAliasesHidden(String nonPrimaryIndex, Set<String> aliasNames) throws InterruptedException, ExecutionException {
private void assertAliasesHidden(String nonPrimaryIndex, Set<String> aliasNames, int aliasCount) throws InterruptedException,
ExecutionException {
final GetAliasesResponse getAliasesResponse = client().admin()
.indices()
.getAliases(new GetAliasesRequest().indicesOptions(IndicesOptions.strictExpandHidden()))
.get();

assertThat(getAliasesResponse.getAliases().size(), equalTo(1));
assertThat(getAliasesResponse.getAliases().get(nonPrimaryIndex).size(), equalTo(2));
assertThat(getAliasesResponse.getAliases().get(nonPrimaryIndex).size(), equalTo(aliasCount));
assertThat(
getAliasesResponse.getAliases().get(nonPrimaryIndex).stream().map(AliasMetadata::alias).collect(Collectors.toSet()),
equalTo(aliasNames)
Expand Down
Loading