-
Notifications
You must be signed in to change notification settings - Fork 25.8k
Update View CRUD Actions to be Index Actions #141570
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
base: main
Are you sure you want to change the base?
Changes from 8 commits
464a468
93a1956
27f3801
2ef9ff4
5cccaea
3ec5ea2
c0c867b
aaf5c60
beddddf
b09d35b
3faf350
3a7709a
eaf9983
47fd65c
ec8db54
e91761b
4bb756f
ab286ff
0342a3d
38af812
0328bc4
1ffbf25
bbfe727
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| area: "Authorization" | ||
| issues: [] | ||
| pr: 141570 | ||
| summary: Update View CRUD Actions to be Index Actions | ||
| type: enhancement |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1851,7 +1851,7 @@ static SortedMap<String, IndexAbstraction> buildIndicesLookup( | |
| ViewMetadata viewMetadata, | ||
| ImmutableOpenMap<String, IndexMetadata> indices | ||
| ) { | ||
| if (indices.isEmpty()) { | ||
| if (indices.isEmpty() && viewMetadata.views().isEmpty()) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the reason that views are included here but data streams are not is that views can exist without indices whereas data streams can't?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I got some strange results when testing views without creating indices first. Since a view can be valid and even return results without referencing an index, we need this check. |
||
| return Collections.emptySortedMap(); | ||
| } | ||
| Map<String, IndexAbstraction> indicesLookup = new HashMap<>(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -86,7 +86,8 @@ public void testToXContent() throws IOException { | |||
| randomBoolean(), | ||||
| randomBoolean(), | ||||
| defaultResolveAliasForThisRequest, | ||||
| randomBoolean() | ||||
| randomBoolean(), | ||||
| false // Specifying views in the create snapshot request is not supported | ||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Specifying views in a restore/create snapshot request is currently not part of the views feature. Not sure if we want to support this?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Views are already part of snapshots in that we capture all views as part of the global state in a snapshot, from elasticsearch/server/src/main/java/org/elasticsearch/cluster/metadata/ViewMetadata.java Line 90 in 8b34252
I think eventually the ESQL team will need to decide how to treat views individually, since they cannot be selected currently other than all-or-nothing.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @craigtaverner FYI might be useful with an issue/task for this. |
||||
| ) | ||||
| ) | ||||
| .gatekeeperOptions(IndicesOptions.GatekeeperOptions.builder().allowSelectors(false).includeFailureIndices(true).build()) | ||||
|
|
||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,8 +30,6 @@ | |
| import java.util.Map; | ||
|
|
||
| import static org.hamcrest.Matchers.containsString; | ||
| import static org.junit.Assert.assertFalse; | ||
| import static org.junit.Assert.assertNotNull; | ||
|
|
||
| public class RestoreSnapshotRequestTests extends AbstractWireSerializingTestCase<RestoreSnapshotRequest> { | ||
| private RestoreSnapshotRequest randomState(RestoreSnapshotRequest instance) { | ||
|
|
@@ -89,7 +87,8 @@ private RestoreSnapshotRequest randomState(RestoreSnapshotRequest instance) { | |
| randomBoolean(), | ||
| randomBoolean(), | ||
| instance.indicesOptions().ignoreAliases() == false, | ||
| randomBoolean() | ||
| randomBoolean(), | ||
| false // Specifying views in the restore snapshot request is not supported | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Specifying views in a restore/create snapshot request is currently not part of the views feature. Not sure if we want to support this? |
||
| ) | ||
| ) | ||
| .gatekeeperOptions(IndicesOptions.GatekeeperOptions.builder().allowSelectors(false).includeFailureIndices(true).build()) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| /* | ||
| * 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.core.esql; | ||
|
|
||
| /** | ||
| * Exposes ES|QL view action names for RBACEngine. | ||
| */ | ||
| public class EsqlViewActionNames { | ||
| public static final String ESQL_PUT_VIEW_ACTION_NAME = "indices:admin/esql/view/put"; | ||
| public static final String ESQL_GET_VIEW_ACTION_NAME = "indices:admin/esql/view/get"; | ||
| public static final String ESQL_DELETE_VIEW_ACTION_NAME = "indices:admin/esql/view/delete"; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,6 +39,7 @@ | |
| import org.elasticsearch.xpack.core.ccr.action.ForgetFollowerAction; | ||
| import org.elasticsearch.xpack.core.ccr.action.PutFollowAction; | ||
| import org.elasticsearch.xpack.core.ccr.action.UnfollowAction; | ||
| import org.elasticsearch.xpack.core.esql.EsqlViewActionNames; | ||
| import org.elasticsearch.xpack.core.ilm.action.ExplainLifecycleAction; | ||
| import org.elasticsearch.xpack.core.inference.action.GetInferenceFieldsInternalAction; | ||
| import org.elasticsearch.xpack.core.rollup.action.GetRollupIndexCapsAction; | ||
|
|
@@ -194,6 +195,10 @@ public final class IndexPrivilege extends Privilege { | |
| "internal:transport/proxy/indices:internal/admin/ccr/restore/session/clear*", | ||
| "internal:transport/proxy/indices:internal/admin/ccr/restore/file_chunk/get*" | ||
| ); | ||
| private static final Automaton CREATE_VIEW_AUTOMATON = patterns(EsqlViewActionNames.ESQL_PUT_VIEW_ACTION_NAME); | ||
| private static final Automaton READ_VIEW_METADATA_AUTOMATON = patterns(EsqlViewActionNames.ESQL_GET_VIEW_ACTION_NAME); | ||
| private static final Automaton DELETE_VIEW_AUTOMATON = patterns(EsqlViewActionNames.ESQL_DELETE_VIEW_ACTION_NAME); | ||
| private static final Automaton MANAGE_VIEW_AUTOMATON = patterns("indices:admin/esql/view*"); | ||
|
|
||
| public static final IndexPrivilege NONE = new IndexPrivilege("none", Automatons.EMPTY); | ||
| public static final IndexPrivilege ALL = new IndexPrivilege("all", ALL_AUTOMATON, IndexComponentSelectorPredicate.ALL); | ||
|
|
@@ -231,6 +236,10 @@ public final class IndexPrivilege extends Privilege { | |
| "cross_cluster_replication_internal", | ||
| CROSS_CLUSTER_REPLICATION_INTERNAL_AUTOMATON | ||
| ); | ||
| public static final IndexPrivilege MANAGE_VIEW = new IndexPrivilege("manage_view", MANAGE_VIEW_AUTOMATON); | ||
| public static final IndexPrivilege CREATE_VIEW = new IndexPrivilege("create_view", CREATE_VIEW_AUTOMATON); | ||
| public static final IndexPrivilege DELETE_VIEW = new IndexPrivilege("delete_view", DELETE_VIEW_AUTOMATON); | ||
| public static final IndexPrivilege READ_VIEW_METADATA = new IndexPrivilege("read_view_metadata", READ_VIEW_METADATA_AUTOMATON); | ||
|
|
||
| public static final IndexPrivilege READ_FAILURE_STORE = new IndexPrivilege( | ||
| "read_failure_store", | ||
|
|
@@ -275,7 +284,11 @@ public final class IndexPrivilege extends Privilege { | |
| entry("maintenance", MAINTENANCE), | ||
| entry("auto_configure", AUTO_CONFIGURE), | ||
| entry("cross_cluster_replication", CROSS_CLUSTER_REPLICATION), | ||
| entry("cross_cluster_replication_internal", CROSS_CLUSTER_REPLICATION_INTERNAL) | ||
| entry("cross_cluster_replication_internal", CROSS_CLUSTER_REPLICATION_INTERNAL), | ||
| entry("manage_view", MANAGE_VIEW), | ||
|
||
| entry("create_view", CREATE_VIEW), | ||
| entry("delete_view", DELETE_VIEW), | ||
| entry("read_view_metadata", READ_VIEW_METADATA) | ||
| ).collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, Map.Entry::getValue)) | ||
| ) | ||
| ); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IndicesOptions.wildcardOptionsdefines how wildcard expressions should be expanded. The newresolveViewswildcard option is added to allow index expressions to be resolved to views by the authorization engine. In addition to that the new flag on the wilcard index option should:writeIndicesOptions) or deserialized (readIndicesOptions) since it's only used for local actions (this may change if delete starts supporting multi-target syntax since the request will be sent to the master node and authorized there).