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.

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

Handle the existence of system data streams in Get Aliases API #73244

Merged
merged 12 commits into from
May 19, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,8 @@ protected ClusterBlockException checkBlock(GetAliasesRequest request, ClusterSta

@Override
protected void masterOperation(Task task, GetAliasesRequest request, ClusterState state, ActionListener<GetAliasesResponse> listener) {
String[] concreteIndices;
// Switch to a context which will drop any deprecation warnings, because there may be indices resolved here which are not
// returned in the final response. We'll add warnings back later if necessary in checkSystemIndexAccess.
try (ThreadContext.StoredContext ignore = threadPool.getThreadContext().newStoredContext(false)) {
concreteIndices = indexNameExpressionResolver.concreteIndexNames(state, request);
}
// resolve all concrete indices upfront and warn/error later
final String[] concreteIndices = indexNameExpressionResolver.concreteIndexNamesWithSystemIndexAccess(state, request);
final SystemIndexAccessLevel systemIndexAccessLevel = indexNameExpressionResolver.getSystemIndexAccessLevel();
ImmutableOpenMap<String, List<AliasMetadata>> aliases = state.metadata().findAliases(request, concreteIndices);
listener.onResponse(new GetAliasesResponse(postProcess(request, concreteIndices, aliases, state,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.common.regex.Regex;
import org.elasticsearch.index.IndexNotFoundException;
import org.elasticsearch.indices.SystemIndices.SystemIndexAccessLevel;

import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -63,7 +64,8 @@ public List<String> resolveIndexAbstractions(Iterable<String> indices, IndicesOp
// continue
indexAbstraction = dateMathName;
} else if (availableIndexAbstractions.contains(dateMathName) &&
isIndexVisible(indexAbstraction, dateMathName, indicesOptions, metadata, includeDataStreams, true)) {
isIndexVisible(indexAbstraction, dateMathName, indicesOptions, metadata, indexNameExpressionResolver,
includeDataStreams, true)) {
if (minus) {
finalIndices.remove(dateMathName);
} else {
Expand All @@ -81,7 +83,8 @@ public List<String> resolveIndexAbstractions(Iterable<String> indices, IndicesOp
Set<String> resolvedIndices = new HashSet<>();
for (String authorizedIndex : availableIndexAbstractions) {
if (Regex.simpleMatch(indexAbstraction, authorizedIndex) &&
isIndexVisible(indexAbstraction, authorizedIndex, indicesOptions, metadata, includeDataStreams)) {
isIndexVisible(indexAbstraction, authorizedIndex, indicesOptions, metadata, indexNameExpressionResolver,
includeDataStreams)) {
resolvedIndices.add(authorizedIndex);
}
}
Expand Down Expand Up @@ -109,12 +112,12 @@ public List<String> resolveIndexAbstractions(Iterable<String> indices, IndicesOp
}

public static boolean isIndexVisible(String expression, String index, IndicesOptions indicesOptions, Metadata metadata,
boolean includeDataStreams) {
return isIndexVisible(expression, index, indicesOptions, metadata, includeDataStreams, false);
IndexNameExpressionResolver resolver, boolean includeDataStreams) {
return isIndexVisible(expression, index, indicesOptions, metadata, resolver, includeDataStreams, false);
}

public static boolean isIndexVisible(String expression, String index, IndicesOptions indicesOptions, Metadata metadata,
boolean includeDataStreams, boolean dateMathExpression) {
IndexNameExpressionResolver resolver, boolean includeDataStreams, boolean dateMathExpression) {
IndexAbstraction indexAbstraction = metadata.getIndicesLookup().get(index);
if (indexAbstraction == null) {
throw new IllegalStateException("could not resolve index abstraction [" + index + "]");
Expand All @@ -127,7 +130,22 @@ public static boolean isIndexVisible(String expression, String index, IndicesOpt
return isVisible && indicesOptions.ignoreAliases() == false;
}
if (indexAbstraction.getType() == IndexAbstraction.Type.DATA_STREAM) {
return isVisible && includeDataStreams;
if (includeDataStreams == false) {
return false;
}

if (indexAbstraction.isSystem()) {
final SystemIndexAccessLevel level = resolver.getSystemIndexAccessLevel();
if (level == SystemIndexAccessLevel.ALL) {
return true;
} else if (level == SystemIndexAccessLevel.NONE) {
return false;
} else if (level == SystemIndexAccessLevel.RESTRICTED) {
return resolver.getSystemIndexAccessPredicate().test(indexAbstraction.getName());
}
} else {
return isVisible;
}
}
assert indexAbstraction.getIndices().size() == 1 : "concrete index must point to a single index";
// since it is a date math expression, we consider the index visible regardless of open/closed/hidden as the user is using
Expand All @@ -139,6 +157,22 @@ public static boolean isIndexVisible(String expression, String index, IndicesOpt
if (isVisible == false) {
return false;
}
if (indexAbstraction.isSystem()) {
// system index that backs system data stream
if (indexAbstraction.getParentDataStream() != null) {
if (indexAbstraction.getParentDataStream().isSystem() == false) {
throw new IllegalStateException("system index is part of a data stream that is not a system data stream");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is a worth candidate for promoting to an assert - we should never hit this case, right?

Copy link
Member

Choose a reason for hiding this comment

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

it should never hit this case but I prefer this actually being detected in the wild since we don't run with assertions on normally

Copy link
Contributor Author

@gwbrown gwbrown May 19, 2021

Choose a reason for hiding this comment

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

++, I meant along the lines of:

assert false : "this means you probably wrote a bug that lets system indices be part of non-system data streams";
throw new IllegalStateException("system index is part of a data stream that is not a system data stream"); 

This shouldn't block the PR though, more maybe as a follow-up?

}
final SystemIndexAccessLevel level = resolver.getSystemIndexAccessLevel();
if (level == SystemIndexAccessLevel.ALL) {
return true;
} else if (level == SystemIndexAccessLevel.NONE) {
return false;
} else if (level == SystemIndexAccessLevel.RESTRICTED) {
return resolver.getSystemIndexAccessPredicate().test(indexAbstraction.getName());
}
}
}

IndexMetadata indexMetadata = indexAbstraction.getIndices().get(0);
if (indexMetadata.getState() == IndexMetadata.State.CLOSE && indicesOptions.expandWildcardsClosed()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,256 @@
/*
* 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.fleet;

import org.apache.http.util.EntityUtils;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.RequestOptions;
import org.elasticsearch.client.Response;
import org.elasticsearch.client.ResponseException;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.test.SecuritySettingsSourceField;
import org.elasticsearch.test.rest.ESRestTestCase;

import java.util.Collections;
import java.util.List;

import static java.util.Collections.emptyList;
import static java.util.Collections.singletonList;
import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;

public class FleetDataStreamIT extends ESRestTestCase {

static final String BASIC_AUTH_VALUE = basicAuthHeaderValue(
"x_pack_rest_user",
SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING
);

@Override
protected Settings restClientSettings() {
// Note that we are superuser here but DO NOT provide a product origin
return Settings.builder().put(ThreadContext.PREFIX + ".Authorization", BASIC_AUTH_VALUE).build();
}

@Override
protected Settings restAdminSettings() {
// Note that we are both superuser here and provide a product origin
return Settings.builder()
.put(ThreadContext.PREFIX + ".Authorization", BASIC_AUTH_VALUE)
.put(ThreadContext.PREFIX + ".X-elastic-product-origin", "fleet")
.build();
}

public void testAliasWithSystemDataStream() throws Exception {
// Create a system data stream
Request initialDocResponse = new Request("POST", ".fleet-actions-results/_doc");
initialDocResponse.setJsonEntity("{\"@timestamp\": 0}");
assertOK(adminClient().performRequest(initialDocResponse));

// Create a system index - this one has an alias
Request sysIdxRequest = new Request("PUT", ".fleet-artifacts");
assertOK(adminClient().performRequest(sysIdxRequest));

// Create a regular index
String regularIndex = "regular-idx";
String regularAlias = "regular-alias";
Request regularIdxRequest = new Request("PUT", regularIndex);
regularIdxRequest.setJsonEntity("{\"aliases\": {\"" + regularAlias + "\": {}}}");
assertOK(client().performRequest(regularIdxRequest));

assertGetAliasAPIBehavesAsExpected(regularIndex, regularAlias);
}

public void testAliasWithSystemIndices() throws Exception {
// Create a system index - this one has an alias
Request sysIdxRequest = new Request("PUT", ".fleet-artifacts");
assertOK(adminClient().performRequest(sysIdxRequest));

// Create a regular index
String regularIndex = "regular-idx";
String regularAlias = "regular-alias";
Request regularIdxRequest = new Request("PUT", regularIndex);
regularIdxRequest.setJsonEntity("{\"aliases\": {\"" + regularAlias + "\": {}}}");
assertOK(client().performRequest(regularIdxRequest));

assertGetAliasAPIBehavesAsExpected(regularIndex, regularAlias);
}

private void assertGetAliasAPIBehavesAsExpected(String regularIndex, String regularAlias) throws Exception {
// Get a non-system alias, should not warn or error
{
Request request = new Request("GET", "_alias/" + regularAlias);
Response response = client().performRequest(request);
assertOK(response);
assertThat(
EntityUtils.toString(response.getEntity()),
allOf(containsString(regularAlias), containsString(regularIndex), not(containsString(".fleet-artifacts")))
);
}

// Fully specify a regular index and alias, should not warn or error
{
Request request = new Request("GET", regularIndex + "/_alias/" + regularAlias);
Response response = client().performRequest(request);
assertOK(response);
assertThat(
EntityUtils.toString(response.getEntity()),
allOf(containsString(regularAlias), containsString(regularIndex), not(containsString(".fleet-artifacts")))
);
}

// The rest of these produce a warning
RequestOptions consumeWarningsOptions = RequestOptions.DEFAULT.toBuilder()
.setWarningsHandler(
warnings -> Collections.singletonList(
"this request accesses system indices: [.fleet-artifacts-7], but "
+ "in a future major version, direct access to system indices will be prevented by default"
).equals(warnings) == false
)
.build();

// The base _alias route warns because there is a system index in the response
{
Request request = new Request("GET", "_alias");
request.setOptions(consumeWarningsOptions); // The result includes system indices, so we warn
Response response = client().performRequest(request);
assertOK(response);
assertThat(
EntityUtils.toString(response.getEntity()),
allOf(containsString(regularAlias), containsString(regularIndex), not(containsString(".fleet-actions-results")))
);
}

// Specify a system alias, should warn
{
Request request = new Request("GET", "_alias/.fleet-artifacts");
request.setOptions(consumeWarningsOptions);
Response response = client().performRequest(request);
assertOK(response);
assertThat(
EntityUtils.toString(response.getEntity()),
allOf(
containsString(".fleet-artifacts"),
containsString(".fleet-artifacts-7"),
not(containsString(regularAlias)),
not(containsString(regularIndex))
)
);
}

// Fully specify a system index and alias, should warn
{
Request request = new Request("GET", ".fleet-artifacts-7/_alias/.fleet-artifacts");
request.setOptions(consumeWarningsOptions);
Response response = client().performRequest(request);
assertOK(response);
assertThat(
EntityUtils.toString(response.getEntity()),
allOf(
containsString(".fleet-artifacts"),
containsString(".fleet-artifacts-7"),
not(containsString(regularAlias)),
not(containsString(regularIndex))
)
);
}

// Check an alias that doesn't exist
{
Request getAliasRequest = new Request("GET", "_alias/auditbeat-7.13.0");
try {
client().performRequest(getAliasRequest);
fail("this request should not succeed, as it is looking for an alias that does not exist");
} catch (ResponseException e) {
assertThat(e.getResponse().getStatusLine().getStatusCode(), is(404));
assertThat(
EntityUtils.toString(e.getResponse().getEntity()),
not(containsString("use and access is reserved for system operations"))
);
}
}

// Specify a system data stream as an alias - should 404
{
Request getAliasRequest = new Request("GET", "_alias/.fleet-actions-results");
try {
client().performRequest(getAliasRequest);
fail("this request should not succeed, as it is looking for an alias that does not exist");
} catch (ResponseException e) {
assertThat(e.getResponse().getStatusLine().getStatusCode(), is(404));
assertThat(
EntityUtils.toString(e.getResponse().getEntity()),
not(containsString("use and access is reserved for system operations"))
);
}
}
}

public void testCountWithSystemDataStream() throws Exception {
assertThatAPIWildcardResolutionWorks();

// Create a system data stream
Request initialDocResponse = new Request("POST", ".fleet-actions-results/_doc");
initialDocResponse.setJsonEntity("{\"@timestamp\": 0}");
assertOK(adminClient().performRequest(initialDocResponse));
assertThatAPIWildcardResolutionWorks();

// Create a system index - this one has an alias
Request sysIdxRequest = new Request("PUT", ".fleet-artifacts");
assertOK(adminClient().performRequest(sysIdxRequest));
assertThatAPIWildcardResolutionWorks(
singletonList(
"this request accesses system indices: [.fleet-artifacts-7], but in a future major version, direct access to system"
+ " indices will be prevented by default"
)
);
assertThatAPIWildcardResolutionWorks(
singletonList(
"this request accesses system indices: [.fleet-artifacts-7], but in a future major version, direct access to system"
+ " indices will be prevented by default"
),
".f*"
);

// Create a regular index
String regularIndex = "regular-idx";
String regularAlias = "regular-alias";
Request regularIdxRequest = new Request("PUT", regularIndex);
regularIdxRequest.setJsonEntity("{\"aliases\": {\"" + regularAlias + "\": {}}}");
assertOK(client().performRequest(regularIdxRequest));
assertThatAPIWildcardResolutionWorks(
singletonList(
"this request accesses system indices: [.fleet-artifacts-7], but in a future major version, direct access to system"
+ " indices will be prevented by default"
)
);
assertThatAPIWildcardResolutionWorks(emptyList(), "r*");
}

private void assertThatAPIWildcardResolutionWorks() throws Exception {
assertThatAPIWildcardResolutionWorks(emptyList(), null);
}

private void assertThatAPIWildcardResolutionWorks(List<String> warningsExpected) throws Exception {
assertThatAPIWildcardResolutionWorks(warningsExpected, null);
}

private void assertThatAPIWildcardResolutionWorks(List<String> warningsExpected, String indexPattern) throws Exception {
String path = indexPattern == null || indexPattern.isEmpty() ? "/_count" : "/" + indexPattern + "/_count";
Request countRequest = new Request("GET", path);
if (warningsExpected.isEmpty() == false) {
countRequest.setOptions(
countRequest.getOptions().toBuilder().setWarningsHandler(warnings -> warningsExpected.equals(warnings) == false)
);
}
assertOK(client().performRequest(countRequest));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ ResolvedIndices resolveIndicesAndAliases(IndicesRequest indicesRequest, Metadata
if (IndexNameExpressionResolver.isAllIndices(indicesList(indicesRequest.indices()))) {
if (replaceWildcards) {
for (String authorizedIndex : authorizedIndices) {
if (IndexAbstractionResolver.isIndexVisible("*", authorizedIndex, indicesOptions, metadata,
if (IndexAbstractionResolver.isIndexVisible("*", authorizedIndex, indicesOptions, metadata, nameExpressionResolver,
indicesRequest.includeDataStreams())) {
resolvedIndicesBuilder.addLocal(authorizedIndex);
}
Expand Down