From 08950ff49172785747483394fde8e9daf0fede88 Mon Sep 17 00:00:00 2001 From: javanna Date: Mon, 4 Dec 2017 14:15:29 +0100 Subject: [PATCH] Remove security filter, replaced by get index api call which returns filtered mappings Given that we get now filtered mappings directly from the get index API (in case security is configured with FLS), we don't need the security filter nor the filtered catalog. That means we can remove the delayed action support also from AuthorizationService and rather make SQLAction a composite action like others. It will be authorized as an action, but its indices won't be checked while that will happen with its inner actions (get index and search) which need to be properly authorized. Also, SQLGetIndicesAction is not needed anymore, as its purpose was to retrieve the indices access resolver put in the context by the security plugin for delayed actions, which are not supported anymore. This commit kind of reverts elastic/x-pack-elasticsearch#2162, as it is now possible to integrate with security out-of-the-box relates elastic/x-pack-elasticsearch#2934 Original commit: elastic/x-pack-elasticsearch@64d504442662a5a0319bbeece724dad3ee2c4826 --- .../org/elasticsearch/xpack/XPackPlugin.java | 6 +- .../security/authz/AuthorizationService.java | 89 +------ .../xpack/sql/SecurityCatalogFilter.java | 96 -------- .../authz/AuthorizationServiceTests.java | 48 ---- .../qa/sql/security/SqlSecurityTestCase.java | 5 +- .../xpack/qa/sql/embed/JdbcProtoHandler.java | 2 +- .../xpack/qa/sql/embed/ProtoHandler.java | 2 +- .../sql/analysis/catalog/FilteredCatalog.java | 38 --- .../sql/analysis/catalog/IndexResolver.java | 21 +- .../sql/plan/logical/command/ShowTables.java | 9 +- .../xpack/sql/plugin/RestSqlJdbcAction.java | 47 +--- .../xpack/sql/plugin/SqlGetIndicesAction.java | 220 ------------------ .../xpack/sql/plugin/SqlPlugin.java | 13 +- .../catalog/FilteredCatalogTests.java | 69 ------ .../sql/plugin/SqlGetIndicesActionTests.java | 70 ------ .../sql/plugin/SqlGetIndicesRequestTests.java | 94 -------- .../xpack/sql/plugin/SqlPluginTests.java | 3 +- 17 files changed, 30 insertions(+), 802 deletions(-) delete mode 100644 plugin/src/main/java/org/elasticsearch/xpack/sql/SecurityCatalogFilter.java delete mode 100644 sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/catalog/FilteredCatalog.java delete mode 100644 sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlGetIndicesAction.java delete mode 100644 sql/server/src/test/java/org/elasticsearch/xpack/sql/analysis/catalog/FilteredCatalogTests.java delete mode 100644 sql/server/src/test/java/org/elasticsearch/xpack/sql/plugin/SqlGetIndicesActionTests.java delete mode 100644 sql/server/src/test/java/org/elasticsearch/xpack/sql/plugin/SqlGetIndicesRequestTests.java diff --git a/plugin/src/main/java/org/elasticsearch/xpack/XPackPlugin.java b/plugin/src/main/java/org/elasticsearch/xpack/XPackPlugin.java index 15a751aca33..a2ca44792bb 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/XPackPlugin.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/XPackPlugin.java @@ -85,8 +85,6 @@ import org.elasticsearch.xpack.rest.action.RestXPackUsageAction; import org.elasticsearch.xpack.security.Security; import org.elasticsearch.xpack.security.authc.AuthenticationService; import org.elasticsearch.xpack.security.authc.support.UsernamePasswordToken; -import org.elasticsearch.xpack.sql.SecurityCatalogFilter; -import org.elasticsearch.xpack.sql.analysis.catalog.FilteredCatalog; import org.elasticsearch.xpack.sql.plugin.SqlLicenseChecker; import org.elasticsearch.xpack.sql.plugin.SqlPlugin; import org.elasticsearch.xpack.ssl.SSLConfigurationReloader; @@ -300,12 +298,10 @@ public class XPackPlugin extends Plugin implements ScriptPlugin, ActionPlugin, I components.addAll(upgrade.createComponents(client, clusterService, threadPool, resourceWatcherService, scriptService, xContentRegistry)); - FilteredCatalog.Filter securityCatalogFilter = XPackSettings.SECURITY_ENABLED.get(settings) ? - new SecurityCatalogFilter(threadPool.getThreadContext(), licenseState) : null; /* Note that we need *client*, not *internalClient* because client preserves the * authenticated user while internalClient throws that user away and acts as the * x-pack user. */ - components.addAll(sql.createComponents(client, securityCatalogFilter)); + components.addAll(sql.createComponents(client)); PersistentTasksExecutorRegistry registry = new PersistentTasksExecutorRegistry(settings, tasksExecutors); PersistentTasksClusterService persistentTasksClusterService = new PersistentTasksClusterService(settings, registry, clusterService); diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java b/plugin/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java index 668d01fdb11..d3f84872508 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java @@ -24,7 +24,6 @@ import org.elasticsearch.action.search.ClearScrollAction; import org.elasticsearch.action.search.MultiSearchAction; import org.elasticsearch.action.search.SearchScrollAction; import org.elasticsearch.action.search.SearchTransportService; -import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.action.support.replication.TransportReplicationAction.ConcreteShardRequest; import org.elasticsearch.action.termvectors.MultiTermVectorsAction; import org.elasticsearch.action.update.UpdateAction; @@ -76,7 +75,6 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.function.BiFunction; import java.util.function.Predicate; import static org.elasticsearch.xpack.security.Security.setting; @@ -86,7 +84,6 @@ public class AuthorizationService extends AbstractComponent { public static final Setting ANONYMOUS_AUTHORIZATION_EXCEPTION_SETTING = Setting.boolSetting(setting("authc.anonymous.authz_exception"), true, Property.NodeScope); public static final String INDICES_PERMISSIONS_KEY = "_indices_permissions"; - public static final String INDICES_PERMISSIONS_RESOLVER_KEY = "_indices_permissions_resolver"; public static final String ORIGINATING_ACTION_KEY = "_originating_action_name"; public static final String ROLE_NAMES_KEY = "_effective_role_names"; @@ -210,70 +207,6 @@ public class AuthorizationService extends AbstractComponent { return; } throw denial(authentication, action, request, permission.names(), null); - } else if (isDelayedIndicesAction(action)) { - /* We check now if the user can execute the action without looking at indices. - * The action is itself responsible for checking if the user can access the - * indices when it runs. */ - if (permission.indices().check(action)) { - grant(authentication, action, request, permission.names(), null); - - /* Now that we know the user can run the action we need to build a function - * that we can use later to fetch the user's actual permissions for an - * index. */ - final MetaData metaData = clusterService.state().metaData(); - final AuthorizedIndices authorizedIndices = new AuthorizedIndices(authentication.getUser(), permission, action, metaData); - - final TransportRequest finalRequest = request; - final Role finalPermission = permission; - setIndicesAccessControlResolver((indicesOptions, indices) -> { - /* The rest of security assumes that it can extract the indices from a - * request and it is fairly twisty to convince it otherwise so we adapt - * and stick our indices into a funny proxy request. Not ideal, but - * it'll do for now. */ - IndicesRequest proxy = new IndicesRequest() { - @Override - public String[] indices() { - return indices; - } - - @Override - public IndicesOptions indicesOptions() { - return indicesOptions; - } - }; - Set specificIndices = new HashSet<>(); - Collections.addAll(specificIndices, indices); - ResolvedIndices resolvedIndices; - try { - resolvedIndices = indicesAndAliasesResolver.resolve(proxy, metaData, authorizedIndices); - } catch (Exception e) { - denial(authentication, action, finalRequest, finalPermission.names(), specificIndices); - throw e; - } - - Set localIndices = new HashSet<>(resolvedIndices.getLocal()); - - IndicesAccessControl indicesAccessControl = finalPermission.authorize(action, localIndices, - metaData, fieldPermissionsCache); - if (!indicesAccessControl.isGranted()) { - throw denial(authentication, action, finalRequest, finalPermission.names(), specificIndices); - } - if (hasSecurityIndexAccess(indicesAccessControl) - && MONITOR_INDEX_PREDICATE.test(action) == false - && isSuperuser(authentication.getUser()) == false) { - // only the superusers are allowed to work with this index, but we should allow indices monitoring actions - // through for debuggingpurposes. These monitor requests also sometimes resolve indices concretely and - // then requests them - logger.debug("user [{}] attempted to directly perform [{}] against the security index [{}]", - authentication.getUser().principal(), action, SecurityLifecycleService.SECURITY_INDEX_NAME); - throw denial(authentication, action, finalRequest, finalPermission.names(), specificIndices); - } - grant(authentication, action, finalRequest, finalPermission.names(), specificIndices); - return indicesAccessControl; - }); - return; - } - throw denial(authentication, action, request, permission.names(), null); } else if (isTranslatedToBulkAction(action)) { if (request instanceof CompositeIndicesRequest == false) { throw new IllegalStateException("Bulk translated actions must implement " + CompositeIndicesRequest.class.getSimpleName() @@ -497,15 +430,6 @@ public class AuthorizationService extends AbstractComponent { } } - /** - * Sets a function to resolve {@link IndicesAccessControl} to be used by - * {@link #isDelayedIndicesAction(String) actions} that do not know their - * indices up front but still need to check permissions. - */ - private void setIndicesAccessControlResolver(BiFunction accessControlResolver) { - putTransientIfNonExisting(INDICES_PERMISSIONS_RESOLVER_KEY, accessControlResolver); - } - private void putTransientIfNonExisting(String key, Object value) { Object existing = threadContext.getTransient(key); if (existing == null) { @@ -558,17 +482,8 @@ public class AuthorizationService extends AbstractComponent { action.equals("indices:data/read/mpercolate") || action.equals("indices:data/read/msearch/template") || action.equals("indices:data/read/search/template") || - action.equals("indices:data/write/reindex"); - } - - /** - * Delayed actions are authorized at start time but do not know which - * indices they target when the start so {@link AuthorizationService} - * sets up a function that can be used to authorize indices during - * the request. - */ - private static boolean isDelayedIndicesAction(String action) { - return action.equals(SqlAction.NAME) || + action.equals("indices:data/write/reindex") || + action.equals(SqlAction.NAME) || action.equals(SqlTranslateAction.NAME); } diff --git a/plugin/src/main/java/org/elasticsearch/xpack/sql/SecurityCatalogFilter.java b/plugin/src/main/java/org/elasticsearch/xpack/sql/SecurityCatalogFilter.java deleted file mode 100644 index df371f5e4f4..00000000000 --- a/plugin/src/main/java/org/elasticsearch/xpack/sql/SecurityCatalogFilter.java +++ /dev/null @@ -1,96 +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; - * you may not use this file except in compliance with the Elastic License. - */ -package org.elasticsearch.xpack.sql; - -import org.elasticsearch.action.support.IndicesOptions; -import org.elasticsearch.common.util.concurrent.ThreadContext; -import org.elasticsearch.license.XPackLicenseState; -import org.elasticsearch.xpack.security.authz.AuthorizationService; -import org.elasticsearch.xpack.security.authz.accesscontrol.IndicesAccessControl; -import org.elasticsearch.xpack.security.authz.accesscontrol.IndicesAccessControl.IndexAccessControl; -import org.elasticsearch.xpack.sql.analysis.catalog.Catalog.GetIndexResult; -import org.elasticsearch.xpack.sql.analysis.catalog.EsIndex; -import org.elasticsearch.xpack.sql.analysis.catalog.FilteredCatalog; -import org.elasticsearch.xpack.sql.type.DataType; - -import java.util.HashMap; -import java.util.Map; -import java.util.function.BiFunction; - -/** - * Adapts SQL to x-pack security by intercepting calls to its catalog and - * authorizing the user's view of every index, filtering out fields that - * the user does not have access to. - *

- * Document level security is handled using the standard search mechanisms - * but this class is required for SQL to respect field level security for - * {@code SELECT} statements and index level security for metadata - * statements like {@code SHOW TABLES} and {@code DESCRIBE TABLE}. - */ -public class SecurityCatalogFilter implements FilteredCatalog.Filter { - private static final IndicesOptions OPTIONS = IndicesOptions.strictSingleIndexNoExpandForbidClosed(); - - private final ThreadContext threadContext; - private final XPackLicenseState licenseState; - - public SecurityCatalogFilter(ThreadContext threadContext, XPackLicenseState licenseState) { - this.threadContext = threadContext; - this.licenseState = licenseState; - } - - @Override - public GetIndexResult filterIndex(GetIndexResult delegateResult) { - if (false == licenseState.isAuthAllowed()) { - /* If security is disabled the index authorization won't be available. - * It is technically possible there to be a race between licensing - * being enabled and sql requests that might cause us to fail on those - * requests but that should be rare. */ - return delegateResult; - } - EsIndex index = delegateResult.get(); - IndicesAccessControl control = getIndicesAccessControl(); - if (control == null) { - // Looks like we're in a delayed response request so lets try that. - BiFunction resolver = getAccessControlResolver(); - if (resolver == null) { - // Looks like we're borked. - throw new IllegalStateException("SQL request wasn't recognized properly by security"); - } - control = resolver.apply(OPTIONS, new String[] {index.name()}); - } - IndexAccessControl permissions = control.getIndexPermissions(index.name()); - /* Fetching the permissions always checks if they are granted. If they aren't - * then it throws an exception. This is ok even for list requests because this - * will only ever be called on indices that are authorized because of security's - * request filtering. */ - if (false == permissions.getFieldPermissions().hasFieldLevelSecurity()) { - return delegateResult; - } - Map filteredMapping = new HashMap<>(index.mapping().size()); - for (Map.Entry entry : index.mapping().entrySet()) { - if (permissions.getFieldPermissions().grantsAccessTo(entry.getKey())) { - filteredMapping.put(entry.getKey(), entry.getValue()); - } - } - return GetIndexResult.valid(new EsIndex(index.name(), filteredMapping)); - } - - /** - * Get the {@link IndicesAccessControl} for this request. This will return null for - * requests that are not indices requests, like SQL's main action. - */ - private IndicesAccessControl getIndicesAccessControl() { - return threadContext.getTransient(AuthorizationService.INDICES_PERMISSIONS_KEY); - } - - /** - * Return a function that resolves permissions to the indices. This will return null - * all actions other than "delayed" actions like the main SQL action. - */ - private BiFunction getAccessControlResolver() { - return threadContext.getTransient(AuthorizationService.INDICES_PERMISSIONS_RESOLVER_KEY); - } -} diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java index 56fc079a64f..ca74888939e 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java @@ -5,7 +5,6 @@ */ package org.elasticsearch.xpack.security.authz; -import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.ElasticsearchSecurityException; import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; @@ -84,7 +83,6 @@ import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; -import org.elasticsearch.common.util.concurrent.ThreadContext.StoredContext; import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.license.GetLicenseAction; @@ -135,9 +133,7 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.UUID; -import java.util.function.BiFunction; -import static java.util.Collections.singleton; import static org.elasticsearch.test.SecurityTestsUtils.assertAuthenticationException; import static org.elasticsearch.test.SecurityTestsUtils.assertThrowsAuthorizationException; import static org.elasticsearch.test.SecurityTestsUtils.assertThrowsAuthorizationExceptionRunAs; @@ -590,33 +586,6 @@ public class AuthorizationServiceTests extends ESTestCase { verify(state, times(1)).metaData(); } - public void testAuditTrailIsRecordedWhenIndexWildcardThrowsErrorDuringDelayed() { - IndicesOptions options = IndicesOptions.fromOptions(false, false, false, false); - TransportRequest request = new SqlRequest(); - ClusterState state = mockEmptyMetaData(); - User user = new User("test user", "a_all"); - RoleDescriptor role = new RoleDescriptor("a_all", null, - new IndicesPrivileges[]{IndicesPrivileges.builder().indices("a").privileges("all").build()}, null); - roleMap.put("a_all", role); - - try (StoredContext context = threadContext.stashContext()) { - authorize(createAuthentication(user), SqlAction.NAME, request); - verify(auditTrail).accessGranted(user, SqlAction.NAME, request, new String[]{ role.getName() }, null); - - BiFunction resolver = - threadContext.getTransient(AuthorizationService.INDICES_PERMISSIONS_RESOLVER_KEY); - final Exception e = expectThrows( - ElasticsearchParseException.class, - () -> resolver.apply(options, new String[] {"<{>"})); - assertEquals("invalid dynamic name expression [{>]. date math placeholder is open ended", e.getMessage()); - verify(auditTrail).accessDenied(user, SqlAction.NAME, request, new String[]{ role.getName() }, singleton("<{>")); - } - - verifyNoMoreInteractions(auditTrail); - verify(clusterService).state(); - verify(state, times(1)).metaData(); - } - public void testRunAsRequestWithNoRolesUser() { TransportRequest request = mock(TransportRequest.class); User user = new User("run as me", null, new User("test user", "admin")); @@ -771,23 +740,6 @@ public class AuthorizationServiceTests extends ESTestCase { verify(auditTrail).accessGranted(user, ClusterHealthAction.NAME, request, new String[] { role.getName() }, null); verifyNoMoreInteractions(auditTrail); - // Delayed request - try (StoredContext context = threadContext.stashContext()) { - request = new SqlRequest(); - authorize(createAuthentication(user), SqlAction.NAME, request); - verify(auditTrail).accessGranted(user, SqlAction.NAME, request, new String[] { role.getName() }, null); - - BiFunction resolver = - threadContext.getTransient(AuthorizationService.INDICES_PERMISSIONS_RESOLVER_KEY); - assertThrowsAuthorizationException( - () -> resolver.apply(IndicesOptions.strictSingleIndexNoExpandForbidClosed(), - new String[] {SecurityLifecycleService.SECURITY_INDEX_NAME}), - SqlAction.NAME, "all_access_user"); - verify(auditTrail).accessDenied(user, SqlAction.NAME, request, new String[] { role.getName() }, - singleton(SecurityLifecycleService.SECURITY_INDEX_NAME)); - verifyNoMoreInteractions(auditTrail); - } - SearchRequest searchRequest = new SearchRequest("_all"); authorize(createAuthentication(user), SearchAction.NAME, searchRequest); assertEquals(2, searchRequest.indices().length); diff --git a/qa/sql/security/src/test/java/org/elasticsearch/xpack/qa/sql/security/SqlSecurityTestCase.java b/qa/sql/security/src/test/java/org/elasticsearch/xpack/qa/sql/security/SqlSecurityTestCase.java index 290e1283612..d4bb306eefc 100644 --- a/qa/sql/security/src/test/java/org/elasticsearch/xpack/qa/sql/security/SqlSecurityTestCase.java +++ b/qa/sql/security/src/test/java/org/elasticsearch/xpack/qa/sql/security/SqlSecurityTestCase.java @@ -143,7 +143,7 @@ public abstract class SqlSecurityTestCase extends ESRestTestCase { } @Before - public void setInitialAuditLogOffset() throws IOException { + public void setInitialAuditLogOffset() { SecurityManager sm = System.getSecurityManager(); if (sm != null) { sm.checkPermission(new SpecialPermission()); @@ -468,9 +468,6 @@ public abstract class SqlSecurityTestCase extends ESRestTestCase { public AuditLogAsserter expectSqlCompositeAction(String user, String... indices) { expect(true, SQL_ACTION_NAME, user, empty()); - for (String index : indices) { - expect(true, SQL_ACTION_NAME, user, hasItems(index)); - } expect(true, GetIndexAction.NAME, user, hasItems(indices)); return this; } diff --git a/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/embed/JdbcProtoHandler.java b/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/embed/JdbcProtoHandler.java index 18b052f817d..8d7a05e4700 100644 --- a/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/embed/JdbcProtoHandler.java +++ b/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/embed/JdbcProtoHandler.java @@ -27,7 +27,7 @@ class JdbcProtoHandler extends ProtoHandler { JdbcProtoHandler(Client client) { super(client); action = new RestSqlJdbcAction(Settings.EMPTY, mock(RestController.class), new SqlLicenseChecker(() -> {}, () -> {}), - new IndexResolver(client, null)); + new IndexResolver(client)); } @Override diff --git a/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/embed/ProtoHandler.java b/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/embed/ProtoHandler.java index 8e85bc2197b..95de8eba204 100644 --- a/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/embed/ProtoHandler.java +++ b/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/embed/ProtoHandler.java @@ -26,7 +26,7 @@ import java.io.IOException; public abstract class ProtoHandler implements HttpHandler, AutoCloseable { private static PlanExecutor planExecutor(EmbeddedModeFilterClient client) { - return new PlanExecutor(client, new IndexResolver(client, null)); + return new PlanExecutor(client, new IndexResolver(client)); } protected static final Logger log = ESLoggerFactory.getLogger(ProtoHandler.class.getName()); diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/catalog/FilteredCatalog.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/catalog/FilteredCatalog.java deleted file mode 100644 index 84acbdd3cb6..00000000000 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/catalog/FilteredCatalog.java +++ /dev/null @@ -1,38 +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; - * you may not use this file except in compliance with the Elastic License. - */ -package org.elasticsearch.xpack.sql.analysis.catalog; - -/** - * {@link Catalog} implementation that filters the results. - */ -public class FilteredCatalog implements Catalog { - public interface Filter { - /** - * Filter an index. Will only be called with valid, - * found indices but gets the entire {@link GetIndexResult} - * from the delegate catalog in case it wants to return - * it unchanged. - */ - GetIndexResult filterIndex(GetIndexResult delegateResult); - } - - private Catalog delegate; - private Filter filter; - - public FilteredCatalog(Catalog delegate, Filter filter) { - this.delegate = delegate; - this.filter = filter; - } - - @Override - public GetIndexResult getIndex(String index) { - GetIndexResult result = delegate.getIndex(index); - if (false == result.isValid() || result.get() == null) { - return result; - } - return filter.filterIndex(result); - } -} diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/catalog/IndexResolver.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/catalog/IndexResolver.java index 2e0b7ee72cd..1fcef8ffe2b 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/catalog/IndexResolver.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/catalog/IndexResolver.java @@ -27,11 +27,9 @@ import java.util.Map; public class IndexResolver { private final Client client; - private final FilteredCatalog.Filter catalogFilter; - public IndexResolver(Client client, FilteredCatalog.Filter catalogFilter) { + public IndexResolver(Client client) { this.client = client; - this.catalogFilter = catalogFilter; } /** @@ -57,18 +55,15 @@ public class IndexResolver { */ results.put(index, buildGetIndexResult(concreteIndex, index, indexMappings.value)); } - Catalog catalog = new PreloadedCatalog(results); - catalog = catalogFilter != null ? new FilteredCatalog(catalog, catalogFilter) : catalog; - listener.onResponse(catalog); + listener.onResponse(new PreloadedCatalog(results)); }, listener::onFailure)); } /** * Discover (multiple) matching indices for a given name. */ - //TODO this method can take a single index pattern once SqlGetIndicesAction is removed - public void asList(ActionListener> listener, String... indices) { - GetIndexRequest getIndexRequest = createGetIndexRequest(indices); + public void asList(String index, ActionListener> listener) { + GetIndexRequest getIndexRequest = createGetIndexRequest(index); client.admin().indices().getIndex(getIndexRequest, ActionListener.wrap(getIndexResponse -> { Map map = new HashMap<>(); for (ObjectObjectCursor> indexMappings : getIndexResponse.getMappings()) { @@ -84,9 +79,7 @@ public class IndexResolver { List results = new ArrayList<>(map.size()); for (GetIndexResult result : map.values()) { if (result.isValid()) { - //as odd as this is, it will go away once mappings are returned filtered - GetIndexResult filtered = catalogFilter != null ? catalogFilter.filterIndex(result) : result; - results.add(filtered.get()); + results.add(result.get()); } } results.sort(Comparator.comparing(EsIndex::name)); @@ -94,10 +87,10 @@ public class IndexResolver { }, listener::onFailure)); } - private static GetIndexRequest createGetIndexRequest(String... indices) { + private static GetIndexRequest createGetIndexRequest(String index) { return new GetIndexRequest() .local(true) - .indices(indices) + .indices(index) .features(Feature.MAPPINGS) //lenient because we throw our own errors looking at the response e.g. if something was not resolved //also because this way security doesn't throw authorization exceptions but rather honours ignore_unavailable diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/plan/logical/command/ShowTables.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/plan/logical/command/ShowTables.java index b8c4c851ecb..62ae21aa84b 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/plan/logical/command/ShowTables.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/plan/logical/command/ShowTables.java @@ -17,7 +17,7 @@ import org.elasticsearch.xpack.sql.tree.Location; import org.elasticsearch.xpack.sql.type.DataTypes; import org.elasticsearch.xpack.sql.util.StringUtils; -import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Objects; @@ -40,18 +40,17 @@ public class ShowTables extends Command { @Override public List output() { - return Arrays.asList(new RootFieldAttribute(location(), "table", DataTypes.KEYWORD)); + return Collections.singletonList(new RootFieldAttribute(location(), "table", DataTypes.KEYWORD)); } @Override public final void execute(SqlSession session, ActionListener listener) { String pattern = Strings.hasText(this.pattern) ? StringUtils.jdbcToEsPattern(this.pattern) : "*"; - - session.indexResolver().asList(ActionListener.wrap(result -> { + session.indexResolver().asList(pattern, ActionListener.wrap(result -> { listener.onResponse(Rows.of(output(), result.stream() .map(t -> singletonList(t.name())) .collect(toList()))); - }, listener::onFailure), pattern); + }, listener::onFailure)); } @Override diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlJdbcAction.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlJdbcAction.java index 56d3e979b36..2f3d81dda81 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlJdbcAction.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlJdbcAction.java @@ -8,7 +8,6 @@ package org.elasticsearch.xpack.sql.plugin; import org.elasticsearch.Version; import org.elasticsearch.action.main.MainAction; import org.elasticsearch.action.main.MainRequest; -import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.client.Client; import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.Settings; @@ -79,7 +78,7 @@ public class RestSqlJdbcAction extends AbstractSqlProtocolRestAction { /** * Actual implementation of the operation */ - public Consumer operation(Request request, Client client) throws IOException { + public Consumer operation(Request request, Client client) { sqlLicenseChecker.checkIfJdbcAllowed(); RequestType requestType = (RequestType) request.requestType(); switch (requestType) { @@ -89,9 +88,9 @@ public class RestSqlJdbcAction extends AbstractSqlProtocolRestAction { response.getVersion().major, response.getVersion().minor, response.getVersion().toString(), response.getBuild().shortHash(), response.getBuild().date()))); case META_TABLE: - return metaTable(client, (MetaTableRequest) request); + return metaTable((MetaTableRequest) request); case META_COLUMN: - return metaColumn(client, (MetaColumnRequest) request); + return metaColumn((MetaColumnRequest) request); case QUERY_INIT: return queryInit(client, (QueryInitRequest) request); case QUERY_PAGE: @@ -101,46 +100,18 @@ public class RestSqlJdbcAction extends AbstractSqlProtocolRestAction { } } - private Consumer metaTable(Client client, MetaTableRequest request) { - //TODO once mappings are filtered this can go directly to the IndexResolver (code commented out below) + private Consumer metaTable(MetaTableRequest request) { String indexPattern = hasText(request.pattern()) ? StringUtils.jdbcToEsPattern(request.pattern()) : "*"; - SqlGetIndicesAction.Request getRequest = new SqlGetIndicesAction.Request(IndicesOptions.lenientExpandOpen(), indexPattern); - getRequest.local(true); - return channel -> client.execute(SqlGetIndicesAction.INSTANCE, getRequest, toActionListener(channel, - response -> new MetaTableResponse(response.indices().stream().map(EsIndex::name).collect(toList())))); - /*String indexPattern = hasText(request.pattern()) ? StringUtils.jdbcToEsPattern(request.pattern()) : "*"; - return channel -> indexResolver.asList(indexPattern, toActionListener(request, channel, list -> + return channel -> indexResolver.asList(indexPattern, toActionListener(channel, list -> new MetaTableResponse(list.stream() .map(EsIndex::name) - .collect(toList()))));*/ + .collect(toList())))); } - private Consumer metaColumn(Client client, MetaColumnRequest request) { - //TODO once mappings are filtered this can go directly to the IndexResolver (code commented out below) + private Consumer metaColumn(MetaColumnRequest request) { String indexPattern = Strings.hasText(request.tablePattern()) ? StringUtils.jdbcToEsPattern(request.tablePattern()) : "*"; Pattern columnMatcher = hasText(request.columnPattern()) ? StringUtils.likeRegex(request.columnPattern()) : null; - - SqlGetIndicesAction.Request getRequest = new SqlGetIndicesAction.Request(IndicesOptions.lenientExpandOpen(), indexPattern); - getRequest.local(true); - return channel -> client.execute(SqlGetIndicesAction.INSTANCE, getRequest, toActionListener(channel, response -> { - List columns = new ArrayList<>(); - for (EsIndex esIndex : response.indices()) { - int pos = 0; - for (Map.Entry entry : esIndex.mapping().entrySet()) { - pos++; - String name = entry.getKey(); - if (columnMatcher == null || columnMatcher.matcher(name).matches()) { - DataType type = entry.getValue(); - // the column size it's actually its precision (based on the Javadocs) - columns.add(new MetaColumnInfo(esIndex.name(), name, type.sqlType(), type.precision(), pos)); - } - } - } - return new MetaColumnResponse(columns); - })); - /*String indexPattern = Strings.hasText(request.tablePattern()) ? StringUtils.jdbcToEsPattern(request.tablePattern()) : "*"; - Pattern columnMatcher = hasText(request.columnPattern()) ? StringUtils.likeRegex(request.columnPattern()) : null; - return channel -> indexResolver.asList(indexPattern, toActionListener(request, channel, esIndices -> { + return channel -> indexResolver.asList(indexPattern, toActionListener(channel, esIndices -> { List columns = new ArrayList<>(); for (EsIndex esIndex : esIndices) { int pos = 0; @@ -155,7 +126,7 @@ public class RestSqlJdbcAction extends AbstractSqlProtocolRestAction { } } return new MetaColumnResponse(columns); - }));*/ + })); } private Consumer queryInit(Client client, QueryInitRequest request) { diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlGetIndicesAction.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlGetIndicesAction.java deleted file mode 100644 index c56890f0996..00000000000 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlGetIndicesAction.java +++ /dev/null @@ -1,220 +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; - * you may not use this file except in compliance with the Elastic License. - */ -package org.elasticsearch.xpack.sql.plugin; - -import org.elasticsearch.action.Action; -import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.ActionRequestValidationException; -import org.elasticsearch.action.ActionResponse; -import org.elasticsearch.action.IndicesRequest; -import org.elasticsearch.action.support.ActionFilters; -import org.elasticsearch.action.support.IndicesOptions; -import org.elasticsearch.action.support.master.MasterNodeReadOperationRequestBuilder; -import org.elasticsearch.action.support.master.MasterNodeReadRequest; -import org.elasticsearch.action.support.master.TransportMasterNodeReadAction; -import org.elasticsearch.client.ElasticsearchClient; -import org.elasticsearch.cluster.ClusterState; -import org.elasticsearch.cluster.block.ClusterBlockException; -import org.elasticsearch.cluster.block.ClusterBlockLevel; -import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; -import org.elasticsearch.cluster.service.ClusterService; -import org.elasticsearch.common.inject.Inject; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.threadpool.ThreadPool; -import org.elasticsearch.transport.TransportService; -import org.elasticsearch.xpack.sql.analysis.catalog.EsIndex; -import org.elasticsearch.xpack.sql.analysis.catalog.IndexResolver; - -import java.io.IOException; -import java.util.Arrays; -import java.util.List; -import java.util.Objects; - -public class SqlGetIndicesAction - extends Action { - public static final SqlGetIndicesAction INSTANCE = new SqlGetIndicesAction(); - public static final String NAME = "indices:data/read/sql/indices"; - - private SqlGetIndicesAction() { - super(NAME); - } - - @Override - public RequestBuilder newRequestBuilder(ElasticsearchClient client) { - return new RequestBuilder(client, this); - } - - @Override - public Response newResponse() { - return new Response(); - } - - public static class Request extends MasterNodeReadRequest implements IndicesRequest.Replaceable { - private IndicesOptions indicesOptions; - private String[] indices; - - /** - * Deserialization and builder ctor. - */ - Request() {} - - /** - * Sensible ctor. - */ - public Request(IndicesOptions indicesOptions, String... indices) { - this.indicesOptions = indicesOptions; - this.indices = indices; - } - - Request(StreamInput in) throws IOException { - super(in); - indicesOptions = IndicesOptions.readIndicesOptions(in); - indices = in.readStringArray(); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - indicesOptions.writeIndicesOptions(out); - out.writeStringArray(indices); - } - - @Override - public ActionRequestValidationException validate() { - return null; - } - - @Override - public String[] indices() { - return indices; - } - - @Override - public Request indices(String... indices) { - this.indices = indices; - return this; - } - - @Override - public IndicesOptions indicesOptions() { - return indicesOptions; - } - - public Request indicesOptions(IndicesOptions indicesOptions) { - this.indicesOptions = indicesOptions; - return this; - } - - @Override - public boolean equals(Object obj) { - if (obj == null || obj.getClass() != getClass()) { - return false; - } - Request other = (Request) obj; - return Arrays.equals(indices, other.indices) - && indicesOptions.equals(other.indicesOptions) - && local == other.local - && masterNodeTimeout.equals(other.masterNodeTimeout) - && getParentTask().equals(other.getParentTask()); - } - - @Override - public int hashCode() { - return Objects.hash(Arrays.hashCode(indices), indicesOptions, local, masterNodeTimeout, getParentTask()); - } - } - - public static class RequestBuilder extends MasterNodeReadOperationRequestBuilder { - public RequestBuilder(ElasticsearchClient client, SqlGetIndicesAction action) { - super(client, action, new Request()); - } - - RequestBuilder setIndicesOptions(IndicesOptions indicesOptions) { - request.indicesOptions(indicesOptions); - return this; - } - - RequestBuilder setIndices(String... indices) { - request.indices(indices); - return this; - } - } - - public static class Response extends ActionResponse { - private List indices; - - /** - * Deserialization ctor. - */ - Response() {} - - public Response(List indices) { - this.indices = indices; - } - - @Override - public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - throw new UnsupportedOperationException("Must be requested locally for now"); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - throw new UnsupportedOperationException("Must be requested locally for now"); - } - - public List indices() { - return indices; - } - } - - public static class TransportAction extends TransportMasterNodeReadAction { - private final IndexResolver indexResolver; - private final SqlLicenseChecker licenseChecker; - - @Inject - public TransportAction(Settings settings, TransportService transportService, - ClusterService clusterService, ThreadPool threadPool, ActionFilters actionFilters, - IndexNameExpressionResolver indexNameExpressionResolver, SqlLicenseChecker licenseChecker, - IndexResolver indexResolver) { - super(settings, NAME, transportService, clusterService, threadPool, actionFilters, - Request::new, indexNameExpressionResolver); - this.licenseChecker = licenseChecker; - this.indexResolver = indexResolver; - } - - @Override - protected String executor() { - // read operation, lightweight... - return ThreadPool.Names.SAME; - } - - @Override - protected Response newResponse() { - return new Response(); - } - - @Override - protected void masterOperation(Request request, ClusterState state, ActionListener listener) { - licenseChecker.checkIfSqlAllowed(); - operation(indexResolver, request, listener); - } - - @Override - protected ClusterBlockException checkBlock(Request request, ClusterState state) { - return state.blocks().indicesBlockedException(ClusterBlockLevel.METADATA_READ, - indexNameExpressionResolver.concreteIndexNames(state, request)); - } - } - - static void operation(IndexResolver indexResolver, Request request, ActionListener listener) { - indexResolver.asList(ActionListener.wrap(results -> listener.onResponse(new Response(results)), listener::onFailure), - request.indices); - } -} \ No newline at end of file diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlPlugin.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlPlugin.java index 85a9c99b11b..1c553c6af7a 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlPlugin.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlPlugin.java @@ -10,7 +10,6 @@ import org.elasticsearch.action.ActionResponse; import org.elasticsearch.client.Client; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.node.DiscoveryNodes; -import org.elasticsearch.common.Nullable; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.IndexScopedSettings; @@ -19,7 +18,6 @@ import org.elasticsearch.common.settings.SettingsFilter; import org.elasticsearch.plugins.ActionPlugin; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestHandler; -import org.elasticsearch.xpack.sql.analysis.catalog.FilteredCatalog; import org.elasticsearch.xpack.sql.analysis.catalog.IndexResolver; import org.elasticsearch.xpack.sql.execution.PlanExecutor; import org.elasticsearch.xpack.sql.plugin.sql.action.SqlAction; @@ -51,17 +49,13 @@ public class SqlPlugin implements ActionPlugin { /** * Create components used by the sql plugin. - * @param catalogFilter if non-null it is a filter for the catalog to apply security */ - public Collection createComponents(Client client, @Nullable FilteredCatalog.Filter catalogFilter) { + public Collection createComponents(Client client) { if (false == enabled) { return emptyList(); } - indexResolver = new IndexResolver(client, catalogFilter); - return Arrays.asList( - sqlLicenseChecker, - indexResolver, - new PlanExecutor(client, indexResolver)); + indexResolver = new IndexResolver(client); + return Arrays.asList(sqlLicenseChecker, indexResolver, new PlanExecutor(client, indexResolver)); } @Override @@ -86,7 +80,6 @@ public class SqlPlugin implements ActionPlugin { } return Arrays.asList(new ActionHandler<>(SqlAction.INSTANCE, TransportSqlAction.class), - new ActionHandler<>(SqlGetIndicesAction.INSTANCE, SqlGetIndicesAction.TransportAction.class), new ActionHandler<>(SqlTranslateAction.INSTANCE, SqlTranslateAction.TransportAction.class)); } } diff --git a/sql/server/src/test/java/org/elasticsearch/xpack/sql/analysis/catalog/FilteredCatalogTests.java b/sql/server/src/test/java/org/elasticsearch/xpack/sql/analysis/catalog/FilteredCatalogTests.java deleted file mode 100644 index 697eea78a78..00000000000 --- a/sql/server/src/test/java/org/elasticsearch/xpack/sql/analysis/catalog/FilteredCatalogTests.java +++ /dev/null @@ -1,69 +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; - * you may not use this file except in compliance with the Elastic License. - */ -package org.elasticsearch.xpack.sql.analysis.catalog; - -import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.xpack.sql.analysis.catalog.Catalog.GetIndexResult; - -import java.util.Arrays; -import java.util.List; - -import static java.util.Collections.singletonMap; -import static java.util.stream.Collectors.toList; - -public class FilteredCatalogTests extends ESTestCase { - public void testGetTypeNoopCatalog() { - Catalog orig = inMemoryCatalog("a", "b", "c"); - Catalog filtered = new FilteredCatalog(orig, delegateResult -> delegateResult); - assertEquals(orig.getIndex("a"), filtered.getIndex("a")); - assertEquals(orig.getIndex("b"), filtered.getIndex("b")); - assertEquals(orig.getIndex("c"), filtered.getIndex("c")); - assertEquals(orig.getIndex("missing"), filtered.getIndex("missing")); - } - - public void testGetTypeFiltering() { - Catalog orig = inMemoryCatalog("cat", "dog"); - Catalog filtered = new FilteredCatalog(orig, delegateResult -> { - if (delegateResult.get().name().equals("cat")) { - return delegateResult; - } - return GetIndexResult.notFound(delegateResult.get().name()); - }); - assertEquals(orig.getIndex("cat"), filtered.getIndex("cat")); - assertEquals(GetIndexResult.notFound("dog"), filtered.getIndex("dog")); - } - - public void testGetTypeModifying() { - Catalog orig = inMemoryCatalog("cat", "dog"); - Catalog filtered = new FilteredCatalog(orig, delegateResult -> { - EsIndex index = delegateResult.get(); - if (index.name().equals("cat")) { - return delegateResult; - } - return GetIndexResult.valid(new EsIndex("rat", index.mapping())); - }); - assertEquals(orig.getIndex("cat"), filtered.getIndex("cat")); - assertEquals("rat", filtered.getIndex("dog").get().name()); - } - - public void testGetTypeFilterIgnoresMissing() { - Catalog orig = inMemoryCatalog(); - Catalog filtered = new FilteredCatalog(orig, delegateResult -> { - if (delegateResult.get().name().equals("cat")) { - return delegateResult; - } - return GetIndexResult.notFound(delegateResult.get().name()); - }); - assertEquals(orig.getIndex("missing"), filtered.getIndex("missing")); - } - - private Catalog inMemoryCatalog(String... indexNames) { - List indices = Arrays.stream(indexNames) - .map(i -> new EsIndex(i, singletonMap("field", null))) - .collect(toList()); - return new InMemoryCatalog(indices); - } -} diff --git a/sql/server/src/test/java/org/elasticsearch/xpack/sql/plugin/SqlGetIndicesActionTests.java b/sql/server/src/test/java/org/elasticsearch/xpack/sql/plugin/SqlGetIndicesActionTests.java deleted file mode 100644 index 8a2efd3e84c..00000000000 --- a/sql/server/src/test/java/org/elasticsearch/xpack/sql/plugin/SqlGetIndicesActionTests.java +++ /dev/null @@ -1,70 +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; - * you may not use this file except in compliance with the Elastic License. - */ -package org.elasticsearch.xpack.sql.plugin; - -import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.support.IndicesOptions; -import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.xpack.sql.analysis.catalog.EsIndex; -import org.elasticsearch.xpack.sql.analysis.catalog.IndexResolver; -import org.mockito.Matchers; -import org.mockito.stubbing.Answer; - -import java.io.IOException; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.concurrent.atomic.AtomicReference; - -import static org.hamcrest.Matchers.hasSize; -import static org.mockito.Matchers.any; -import static org.mockito.Mockito.doAnswer; -import static org.mockito.Mockito.mock; - -public class SqlGetIndicesActionTests extends ESTestCase { - - public void testOperation() throws IOException, InterruptedException { - SqlGetIndicesAction.Request request = new SqlGetIndicesAction.Request(IndicesOptions.lenientExpandOpen(), "test", "bar", "foo*"); - - List esIndices = new ArrayList<>(); - esIndices.add(new EsIndex("foo1", Collections.emptyMap())); - esIndices.add(new EsIndex("foo2", Collections.emptyMap())); - esIndices.add(new EsIndex("test", Collections.emptyMap())); - - final AtomicReference responseRef = new AtomicReference<>(); - final AtomicReference errorRef = new AtomicReference<>(); - - ActionListener listener = new ActionListener() { - @Override - public void onResponse(SqlGetIndicesAction.Response response) { - responseRef.set(response); - } - - @Override - public void onFailure(Exception e) { - errorRef.set(e); - } - }; - - IndexResolver indexResolver = mock(IndexResolver.class); - doAnswer((Answer) invocationOnMock -> { - @SuppressWarnings("unchecked") - final ActionListener> actionListener = (ActionListener>)invocationOnMock.getArguments()[0]; - actionListener.onResponse(esIndices); - return null; - }).when(indexResolver).asList(any(), Matchers.anyVararg()); - - SqlGetIndicesAction.operation(indexResolver, request, listener); - - assertNull(errorRef.get()); - assertNotNull(responseRef.get()); - SqlGetIndicesAction.Response response = responseRef.get(); - assertThat(response.indices(), hasSize(3)); - assertEquals("foo1", response.indices().get(0).name()); - assertEquals("foo2", response.indices().get(1).name()); - assertEquals("test", response.indices().get(2).name()); - } -} diff --git a/sql/server/src/test/java/org/elasticsearch/xpack/sql/plugin/SqlGetIndicesRequestTests.java b/sql/server/src/test/java/org/elasticsearch/xpack/sql/plugin/SqlGetIndicesRequestTests.java deleted file mode 100644 index b38ccdb3b9d..00000000000 --- a/sql/server/src/test/java/org/elasticsearch/xpack/sql/plugin/SqlGetIndicesRequestTests.java +++ /dev/null @@ -1,94 +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; - * you may not use this file except in compliance with the Elastic License. - */ -package org.elasticsearch.xpack.sql.plugin; - -import org.elasticsearch.action.support.IndicesOptions; -import org.elasticsearch.common.io.stream.Writeable.Reader; -import org.elasticsearch.common.unit.TimeValue; -import org.elasticsearch.tasks.TaskId; -import org.elasticsearch.test.AbstractWireSerializingTestCase; -import org.elasticsearch.xpack.sql.plugin.SqlGetIndicesAction.Request; - -import java.io.IOException; -import java.util.Arrays; -import java.util.function.Supplier; - -public class SqlGetIndicesRequestTests extends AbstractWireSerializingTestCase { - @Override - protected Request createTestInstance() { - Request request = new Request(randomIndicesOptions(), randomIndices()); - request.local(randomBoolean()); - request.masterNodeTimeout(randomTimeValue()); - request.setParentTask(randomTaskId()); - return request; - } - - @Override - protected Reader instanceReader() { - return Request::new; - } - - @Override - protected Request mutateInstance(Request request) throws IOException { - @SuppressWarnings("unchecked") - Supplier supplier = randomFrom( - () -> { - Request mutant = new Request( - randomValueOtherThan(request.indicesOptions(), SqlGetIndicesRequestTests::randomIndicesOptions), - request.indices()); - mutant.local(request.local()); - mutant.masterNodeTimeout(request.masterNodeTimeout()); - mutant.setParentTask(request.getParentTask()); - return mutant; - }, - () -> { - Request mutant = new Request( - request.indicesOptions(), - randomValueOtherThanMany(i -> Arrays.equals(request.indices(), i), SqlGetIndicesRequestTests::randomIndices)); - mutant.local(request.local()); - mutant.masterNodeTimeout(request.masterNodeTimeout()); - mutant.setParentTask(request.getParentTask()); - return mutant; - }, () -> { - Request mutant = new Request(request.indicesOptions(), request.indices()); - mutant.local(false == request.local()); - mutant.masterNodeTimeout(request.masterNodeTimeout()); - mutant.setParentTask(request.getParentTask()); - return mutant; - }, () -> { - Request mutant = new Request(request.indicesOptions(), request.indices()); - mutant.local(request.local()); - mutant.masterNodeTimeout(randomValueOtherThan(request.masterNodeTimeout(), - () -> TimeValue.parseTimeValue(randomTimeValue(), "test"))); - mutant.setParentTask(request.getParentTask()); - return mutant; - }, () -> { - Request mutant = new Request(request.indicesOptions(), request.indices()); - mutant.local(false == request.local()); - mutant.masterNodeTimeout(request.masterNodeTimeout()); - mutant.setParentTask(randomValueOtherThan(request.getParentTask(), SqlGetIndicesRequestTests::randomTaskId)); - return mutant; - }); - return supplier.get(); - } - - private static IndicesOptions randomIndicesOptions() { - return IndicesOptions.fromOptions(randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean(), - randomBoolean(), randomBoolean()); - } - - private static String[] randomIndices() { - String[] indices = new String[between(1, 10)]; - for (int i = 0; i < indices.length; i++) { - indices[i] = randomAlphaOfLength(5); - } - return indices; - } - - private static TaskId randomTaskId() { - return randomBoolean() ? TaskId.EMPTY_TASK_ID : new TaskId(randomAlphaOfLength(5), randomLong()); - } -} diff --git a/sql/server/src/test/java/org/elasticsearch/xpack/sql/plugin/SqlPluginTests.java b/sql/server/src/test/java/org/elasticsearch/xpack/sql/plugin/SqlPluginTests.java index 2200500c1df..801ad86ea46 100644 --- a/sql/server/src/test/java/org/elasticsearch/xpack/sql/plugin/SqlPluginTests.java +++ b/sql/server/src/test/java/org/elasticsearch/xpack/sql/plugin/SqlPluginTests.java @@ -14,7 +14,6 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.SettingsFilter; import org.elasticsearch.rest.RestController; import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.xpack.sql.analysis.catalog.FilteredCatalog; import java.util.Collections; @@ -25,7 +24,7 @@ public class SqlPluginTests extends ESTestCase { public void testSqlDisabled() { SqlPlugin plugin = new SqlPlugin(false, new SqlLicenseChecker(() -> {}, () -> {})); - assertThat(plugin.createComponents(mock(Client.class), mock(FilteredCatalog.Filter.class)), empty()); + assertThat(plugin.createComponents(mock(Client.class)), empty()); assertThat(plugin.getActions(), empty()); assertThat(plugin.getRestHandlers(Settings.EMPTY, mock(RestController.class), new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS),