Tidying up

Signed-off-by: Peter Nied <[email protected]>

server/src/main/java/org/opensearch/action/ResourceRequest.java

@@ -1,40 +1,68 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
 package org.opensearch.action;
 
-import static org.opensearch.action.ValidateActions.addValidationError;
+import org.opensearch.common.annotation.ExperimentalApi;
 
 import java.util.Map;
 import java.util.regex.Pattern;
 
-import org.opensearch.common.annotation.ExperimentalApi;
+import static org.opensearch.action.ValidateActions.addValidationError;
 
 /**
  * TODO: This validation should be associated with REST requests to ensure the parameter is from the URL
  * Note; this should work well in RestHandlers
- * 
+ *
  * Context: If all of the resource information is in the URL, caching which can include responses or authorization are trivial
  */
 @ExperimentalApi
 public interface ResourceRequest {
 
     static final Pattern ALLOWED_RESOURCE_NAME_PATTERN = Pattern.compile("[a-zA-Z_-]+");
-    /** 
+    /**
      * Don't allow wildcard patterns
      * this has large impact to perf and cachability */
     static final Pattern ALLOWED_RESOURCE_ID_PATTERN = Pattern.compile("[a-zA-Z_-]+");
 
+    /** Returns the resource types and ids associated with this request */
+    Map<String, String> getResourceTypeAndIds();
+
     /**
      * Validates the resource type and id pairs are in an allowed format
      */
-    Map<String, String> getResourceTypeAndIds();
-
-    public static ActionRequestValidationException validResourceIds(final ResourceRequest resourceRequest, final ActionRequestValidationException validationException) {
+    public static ActionRequestValidationException validResourceIds(
+        final ResourceRequest resourceRequest,
+        final ActionRequestValidationException validationException
+    ) {
         resourceRequest.getResourceTypeAndIds().entrySet().forEach(entry -> {
             if (!ALLOWED_RESOURCE_NAME_PATTERN.matcher(entry.getKey()).matches()) {
-                addValidationError("Unsupported resource key is not supported; key: " + entry.getKey() + " value: "  + entry.getValue() + " allowed pattern " + ALLOWED_RESOURCE_NAME_PATTERN.pattern(), validationException);
+                addValidationError(
+                    "Unsupported resource key is not supported; key: "
+                        + entry.getKey()
+                        + " value: "
+                        + entry.getValue()
+                        + " allowed pattern "
+                        + ALLOWED_RESOURCE_NAME_PATTERN.pattern(),
+                    validationException
+                );
             }
 
             if (!ALLOWED_RESOURCE_ID_PATTERN.matcher(entry.getKey()).matches()) {
-                addValidationError("Unsupported resource value is not supported; key: " + entry.getKey() + " value: "  + entry.getValue() + " allowed pattern " + ALLOWED_RESOURCE_ID_PATTERN.pattern(), validationException);
+                addValidationError(
+                    "Unsupported resource value is not supported; key: "
+                        + entry.getKey()
+                        + " value: "
+                        + entry.getValue()
+                        + " allowed pattern "
+                        + ALLOWED_RESOURCE_ID_PATTERN.pattern(),
+                    validationException
+                );
             }
         });
 

server/src/main/java/org/opensearch/action/search/ViewSearchRequest.java

@@ -6,50 +6,20 @@
  * compatible open source license.
  */
 
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-/*
- * Modifications Copyright OpenSearch Contributors. See
- * GitHub history for details.
- */
-
 package org.opensearch.action.search;
 
-import static org.opensearch.action.ValidateActions.addValidationError;
-
-import java.io.IOException;
-import java.util.Map;
-import java.util.Objects;
-import java.util.function.Function;
-import java.util.regex.Pattern;
-
 import org.opensearch.action.ActionRequestValidationException;
 import org.opensearch.action.ResourceRequest;
 import org.opensearch.cluster.metadata.View;
-import org.opensearch.common.ValidationException;
-import org.opensearch.common.annotation.ExperimentalApi;
-import org.opensearch.core.common.io.stream.StreamInput;
+import org.opensearch.common.at org.opensearch.core.common.io.stream.StreamInput;
 import org.opensearch.core.common.io.stream.StreamOutput;
-import org.opensearch.rest.action.admin.indices.RestViewAction;
+import org.opensearch.rest.action.admin.indicport java.util.Map;
+import java.util.Objects;
+import java.util.function.Function;
+
+import static org.opensearch.action.ValidateActions.addValidationError;
 
-/** Wraps the functionality of search requests and tailors for what is available when searching through views 
+/** Wraps the functionality of search requests and tailors for what is available when searching through views
  */
 @ExperimentalApi
 public class ViewSearchRequest extends SearchRequest implements ResourceRequest {
@@ -66,7 +36,6 @@ public ViewSearchRequest(final StreamInput in) throws IOException {
         view = new View(in);
     }
 
-
     @Override
     public ActionRequestValidationException validate() {
         final Function<String, String> unsupported = (String x) -> x + " is not supported when searching views";
@@ -83,7 +52,6 @@ public ActionRequestValidationException validate() {
         return validationException;
     }
 
-
     @Override
     public void writeTo(final StreamOutput out) throws IOException {
         super.writeTo(out);

server/src/main/java/org/opensearch/cluster/metadata/View.java

@@ -1,3 +1,10 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
 
 package org.opensearch.cluster.metadata;
 

server/src/main/java/org/opensearch/cluster/metadata/ViewMetadata.java

@@ -1,3 +1,11 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
 package org.opensearch.cluster.metadata;
 
 import org.opensearch.Version;

server/src/main/java/org/opensearch/index/view/views-design.md

@@ -5,6 +5,44 @@ Views define how searches are performed against indices on a cluster, uniform da
 
 ## Design
 
+### View data
+
+Views create a mapping to the resources that hold information to be searched over in a consistent manner.  This abstraction allows for indirection with the backing indices, so they might be changed without callers being impacted.  This can also be used to simplify the security model - searches over views do not require permissions to the backing indices only permissions to the view itself.
+
+```mermaid
+classDiagram
+    class View {
+        +String name
+        +String description
+        +long createdAt
+        +long modifiedAt
+        +List<Target> targets
+        +toXContent(XContentBuilder, Params) XContentBuilder
+        +writeTo(StreamOutput) void
+    }
+    class Target {
+        +String indexPattern
+        +toXContent(XContentBuilder, Params) XContentBuilder
+        +writeTo(StreamOutput) void
+    }
+    class StreamOutput
+    class XContentBuilder
+
+    View  -- Target : contains
+    View -- StreamOutput : writes to
+    View -- XContentBuilder : outputs to
+    Target -- StreamOutput : writes to
+    Target -- XContentBuilder : outputs to
+```
+
+### View persistence
+
+Views are long lived objects in OpenSearch, all operations on them should be fully committed before responding to the caller.  Views are intentionally created for user scenarios following a similar creation cadence to indices.
+
+Committed implies that the updates are synchronized across all nodes in a cluster.  The Cluster Metadata Store is already available and allows for acknowledging that changes have been applied to all nodes.  While this data could be stored in a new purpose built index, index data replication has delays and ensuring synchronization is non-trivial to implement as is seen in the Security plugins [1].
+
+- [1] https://github.com/opensearch-project/security/issues/3275
+
 ```mermaid
 sequenceDiagram
     participant Client
@@ -30,11 +68,63 @@ sequenceDiagram
     HTTP_Request-->>Client: Return
 ```
 
-### Frequently Asked Questions
+### Resource Request
+
+In order to permissions views OpenSearch needs a way to consistently refer to them, this is a generic problem and views will be a first use case.  Resource requests require a map of types to identifiers for the request, multiple resources could be part of a single request, but only one of each type.
+
+Considering the request to search a view, `POST /view/{view_id}/_search`, the path parameter 'view_id' is the type and the value from the request would be the identifier.  
+
+```java
+public interface ResourceRequest {
+   /** Returns the resource types and ids associated with this request */
+    Map<String, String> getResourceTypeAndIds();
+
+    /** Validates the resource type and id pairs are in an allowed format */
+    public static ActionRequestValidationException validResourceIds(
+        final ResourceRequest resourceRequest,
+        final ActionRequestValidationException validationException
+    ) {;}
+}
+```
+
+### Resource Permission Grants
+With requests include resource type and identifiers the security plugin will need to allow for grants to these new types.  Modify the security role to include this information so it can be checked and then the request can be permitted.
+
+```yaml
+all_access:
+  reserved: true
+  hidden: false
+  static: true
+  description: "Allow full access to all indices and all cluster APIs"
+  cluster_permissions:
+    - "*"
+  index_permissions:
+    - index_patterns:
+        - "*"
+      allowed_actions:
+        - "*"
+  tenant_permissions:
+    - tenant_patterns:
+        - "*"
+      allowed_actions:
+        - "kibana_all_write"
+  resource_permissions:
+    - resource_type: "view"
+      resource_ids: ["songs", "albums"]
+```
+
+## Frequently Asked Questions
+
+### How do views work with fine grain access control of index data?
+*To be determined...*
 
-#### How do views work with fine grain access control of index data?
+### What happens with existing DLS and FLS rules and searches on views? 
+*To be determined...*
 
-#### What happens with existing DLS and FLS rules and searches on views? 
+### Additional Question(s)
+*To be determined...*
+
+## Appendix
 
 ### Local Testing
 
@@ -60,7 +150,7 @@ curl localhost:9200/views \
 curl localhost:9200/views/hi/_search
 ```
 
-## Appendix
+### v0 View Data Model
 
 ```
 VIEW MODEL
@@ -85,7 +175,7 @@ VIEW MODEL
 }
 ```
 
-### View Operations
+#### View Operations
 
 | Method | Path |
 | - | - |
@@ -95,25 +185,25 @@ VIEW MODEL
 | PATCH  | /views/{view_id} | 
 | DELETE | /views/{view_id} |
 
-### Enumerate Views
+#### Enumerate Views
 
 | Method | Path |
 | - | - |
 | GET | /views |
 
-### Perform a Search on a view
+#### Perform a Search on a view
 | Method | Path |
 | - | - |
 | GET  | /views/{view_id}/_search |
 | POST | /views/{view_id}/_search |
 
-### Search Views // P2?
+#### Search Views // P2?
 | Method | Path |
 | - | - |
 | GET  | /views/_search |
 | POST | /views/_search |
 
-### Mapping // P2?  Need to understand the utility / impact of not having this
+#### Mapping // P2?  Need to understand the utility / impact of not having this
 | Method | Path |
 | - | - |
 | GET   | /views/{view_id}/_mappings |
@@ -123,7 +213,7 @@ VIEW MODEL
 
 *Results do not include any fields '_', how to protect leaking data?*
 
-### Response on Create/Enumerate/Search
+#### Response on Create/Enumerate/Search
 
 views: [
    {
@@ -133,45 +223,4 @@ views: [
       created: DATE,
       modified: DATE
    }
-]
-
-
-## Permissions
-
-Views can be permissioned directly by name or id.  User must also have READ permission for all indices targeted by the view, authorized before any of these operations.
-
-index:views.view.read[:{view_id}]
-index:views.view.modify[:{view_id}]
-index:views.view.delete[:{view_id}]
-
-Searching with a view. Read permission of the view is *not* required.
-
-index:views.view.search[:{view_id}]
-
-Creation / Enumeration / Searching views is supported.  Only the view name and view id are returned.  Create view call requires READ permission for all indices targeted by the view.
-
-index:views.enumerate
-index:views.search
-index:views.create 
-
-### Internal functionality
-
-Search does not expose information about the targets or associated mappings.  Internally used to resolve all indices and check permissions.
-
-```
-GET /views/{view_id}/_resolveTargets
-{
-targets: [
-   {
-      index: STRING,
-      fields: [
-         {
-            fieldName: STRING
-         }
-      ],
-      _view: VIEW_MIN_OBJECT
-   }, ...
-],
-fields: [ FIELD_NAME,... ]
-}
-```
+]