Skip to content

Commit

Permalink
Correct security plugin operations that take lists or return maps. (o…
Browse files Browse the repository at this point in the history
…pensearch-project#127)

The aws.protocols#restJson1 protocol explicitly disallows lists and maps for @httpPayload.

Requires workaround due to AbstractRestProtocol being package-private, and a bug in HttpBindingIndex waiting on PR smithy-lang/smithy#1840

Signed-off-by: Thomas Farr <tsfarr@amazon.com>
Signed-off-by: Alen Abeshov <a_abeshov@kbtu.kz>
  • Loading branch information
Xtansia authored and aabeshov committed Jul 16, 2023
1 parent 842a952 commit af61538
Show file tree
Hide file tree
Showing 28 changed files with 501 additions and 75 deletions.
2 changes: 1 addition & 1 deletion buildSrc/src/main/kotlin/versions.kt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
const val smithyVersion = "1.31.0"
const val smithyVersion = "1.33.0"
4 changes: 2 additions & 2 deletions model/opensearch.smithy
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@
$version: "2"
namespace OpenSearch

use aws.protocols#restJson1
use opensearch.openapi#restJson

@externalDocumentation(
"OpenSearch Documentation": "https://opensearch.org/docs/latest/"
)

@httpBasicAuth
@restJson1
@restJson
service OpenSearch {
version: "2021-11-23",
operations: [
Expand Down
1 change: 1 addition & 0 deletions model/security/get_account_details/structures.smithy
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,6 @@ structure GetAccountDetails_Input{}

@output
structure GetAccountDetails_Output {
@httpPayload
content: AccountDetails
}
3 changes: 2 additions & 1 deletion model/security/get_role/structures.smithy
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,6 @@ structure GetRole_Input{

@output
structure GetRole_Output {
role: Role
@httpPayload
content: RolesMap
}
7 changes: 2 additions & 5 deletions model/security/get_roles/structures.smithy
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,11 @@
$version: "2"
namespace OpenSearch

list RolesList{
member: Role
}

@input
structure GetRoles_Input {}

@output
structure GetRoles_Output {
content: RolesList
@httpPayload
content: RolesMap
}
3 changes: 2 additions & 1 deletion model/security/get_tenant/structures.smithy
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,6 @@ structure GetTenant_Input{

@output
structure GetTenant_Output {
tenant: Tenant
@httpPayload
content: TenantsMap
}
3 changes: 2 additions & 1 deletion model/security/get_tenants/structures.smithy
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,6 @@ structure GetTenants_Input {}

@output
structure GetTenants_Output {
tenantlist: TenantList
@httpPayload
content: TenantsMap
}
3 changes: 2 additions & 1 deletion model/security/get_user/structures.smithy
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,6 @@ structure GetUser_Input{

@output
structure GetUser_Output {
user: User
@httpPayload
content: UsersMap
}
3 changes: 2 additions & 1 deletion model/security/get_users/structures.smithy
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,6 @@ structure GetUsers_Input {}

@output
structure GetUsers_Output {
content: UserList
@httpPayload
content: UsersMap
}
18 changes: 18 additions & 0 deletions model/security/patch.smithy
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// 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.

$version: "2"
namespace OpenSearch

structure PatchOperation {
op: String,
path: String,
value: Document
}

list PatchOperationList {
member: PatchOperation
}
2 changes: 1 addition & 1 deletion model/security/patch_role/structures.smithy
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ structure PatchRole_Input{
role: String
@required
@httpPayload
role_patch: PatchRoleParams
content: PatchOperationList
}

@output
Expand Down
2 changes: 1 addition & 1 deletion model/security/patch_roles/structures.smithy
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ namespace OpenSearch
structure PatchRoles_Input {
@required
@httpPayload
role_patch: PatchRolesParams
content: PatchOperationList
}

@output
Expand Down
3 changes: 2 additions & 1 deletion model/security/patch_tenant/structures.smithy
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ structure PatchTenant_Input {
@required
@httpLabel
tenant: String
@required
@httpPayload
content: PatchTenantParams
content: PatchOperationList
}

@output
Expand Down
3 changes: 2 additions & 1 deletion model/security/patch_tenants/structures.smithy
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ namespace OpenSearch

@input
structure PatchTenants_Input{
@required
@httpPayload
content: PatchTenantsParams
content: PatchOperationList
}

@output
Expand Down
2 changes: 1 addition & 1 deletion model/security/patch_user/structures.smithy
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ structure PatchUser_Input {
username: String
@required
@httpPayload
content: PatchUserParams
content: PatchOperationList
}

@output
Expand Down
2 changes: 1 addition & 1 deletion model/security/patch_users/structures.smithy
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ namespace OpenSearch
structure PatchUsers_Input{
@required
@httpPayload
content: PatchUsersParams
content: PatchOperationList
}

@output
Expand Down
23 changes: 8 additions & 15 deletions model/security/role.smithy
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
$version: "2"
namespace OpenSearch

structure Role{
structure Role {
reserved: Boolean
hidden: Boolean
description: String
Expand All @@ -28,34 +28,27 @@ structure IndexPermission {
allowed_actions: AllowedActions
}

list RolesList{
member: Role
}

list TenantPermission {
member: String
}

list IndexPatterns{
list IndexPatterns {
member: String
}

list Fls{
list Fls {
member: String
}

list MaskedFields{
list MaskedFields {
member: String
}

list AllowedActions{
list AllowedActions {
member: String
}

structure PatchRoleParams{
rolePatch: PatchOperation
}

structure PatchRolesParams{
rolesPatch: PatchOperationList
map RolesMap {
key: String
value: Role
}
30 changes: 4 additions & 26 deletions model/security/tenant.smithy
Original file line number Diff line number Diff line change
Expand Up @@ -7,36 +7,14 @@
$version: "2"
namespace OpenSearch

map AttributeMap {
key: String,
value: String
}

structure PatchOperation {
op: String,
path: String,
value: AttributeMap
}

structure Tenant{
structure Tenant {
reserved: Boolean,
hidden: Boolean,
description: String,
static: Boolean
}

list TenantList{
member: Tenant
}

structure PatchTenantParams{
tenantPatch: PatchOperation
}

structure PatchTenantsParams{
tenantsPatch: PatchOperationList
}

list PatchOperationList{
member: PatchOperation
map TenantsMap {
key: String,
value: Tenant
}
20 changes: 9 additions & 11 deletions model/security/user.smithy
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,12 @@
$version: "2"
namespace OpenSearch

list UserList {
member: User
}

structure User {
hash: String,
reserved: Boolean,
hidden: Boolean,
backend_roles: BackendRolesList,
attributes: AttributeMap,
attributes: AttributesMap,
description: String,
opendistro_security_roles: OpendistroSecurityRolesList,
static: Boolean
Expand All @@ -26,14 +22,16 @@ list BackendRolesList {
member: String
}

list OpendistroSecurityRolesList {
member: String
map AttributesMap {
key: String,
value: String
}

structure PatchUserParams{
userPatch: PatchOperation
list OpendistroSecurityRolesList {
member: String
}

structure PatchUsersParams{
usersPatch: PatchOperationList
map UsersMap {
key: String,
value: User
}
7 changes: 6 additions & 1 deletion openapi-traits/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@ repositories {
}

dependencies {
implementation("software.amazon.smithy:smithy-openapi:1.31.0")
implementation("software.amazon.smithy:smithy-openapi:1.33.0")
}

java {
sourceCompatibility = JavaVersion.VERSION_11
targetCompatibility = JavaVersion.VERSION_11
}

spotless {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,21 @@

import org.opensearch.smithy.openapi.extensions.mappers.VendorExtensionsJsonSchemaMapper;
import org.opensearch.smithy.openapi.extensions.mappers.VendorExtensionsOpenApiMapper;
import org.opensearch.smithy.openapi.protocols.OpenSearchRestJsonProtocol;
import software.amazon.smithy.jsonschema.JsonSchemaMapper;
import software.amazon.smithy.model.traits.Trait;
import software.amazon.smithy.openapi.fromsmithy.OpenApiMapper;
import software.amazon.smithy.openapi.fromsmithy.OpenApiProtocol;
import software.amazon.smithy.openapi.fromsmithy.Smithy2OpenApiExtension;

import java.util.List;

public class VendorExtensionsExtension implements Smithy2OpenApiExtension {
public class OpenSearchOpenApiExtension implements Smithy2OpenApiExtension {
@Override
public List<OpenApiProtocol<? extends Trait>> getProtocols() {
return List.of(new OpenSearchRestJsonProtocol());
}

@Override
public List<JsonSchemaMapper> getJsonSchemaMappers() {
return List.of(new VendorExtensionsJsonSchemaMapper());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package org.opensearch.smithy.openapi.protocols;

import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
import org.opensearch.smithy.openapi.traits.RestJsonTrait;
import software.amazon.smithy.jsonschema.Schema;
import software.amazon.smithy.model.knowledge.HttpBinding;
import software.amazon.smithy.model.node.Node;
import software.amazon.smithy.model.shapes.Shape;
import software.amazon.smithy.model.shapes.ShapeId;
import software.amazon.smithy.model.shapes.StructureShape;
import software.amazon.smithy.openapi.fromsmithy.Context;
import software.amazon.smithy.openapi.fromsmithy.protocols.AbstractOSRestProtocol;

public final class OpenSearchRestJsonProtocol extends AbstractOSRestProtocol<RestJsonTrait> {
@Override
public Class<RestJsonTrait> getProtocolType() {
return RestJsonTrait.class;
}

@Override
protected String getDocumentMediaType(Context<RestJsonTrait> context, Shape operationOrError, OSMessageType message) {
return context.getConfig().getJsonContentType();
}

@Override
protected Schema createDocumentSchema(Context<RestJsonTrait> context, Shape operationOrError, List<HttpBinding> bindings, OSMessageType messageType) {
if (bindings.isEmpty()) {
return Schema.builder().type("object").build();
}

// We create a synthetic structure shape that is passed through the
// JSON schema converter. This shape only contains members that make
// up the "document" members of the input/output/error shape.
ShapeId container = bindings.get(0).getMember().getContainer();
StructureShape containerShape = context.getModel().expectShape(container, StructureShape.class);

// Path parameters of requests are handled in "parameters" and headers are
// handled in headers, so this method must ensure that only members that
// are sent in the document payload are present in the structure when it is
// converted to OpenAPI. This ensures that any path parameters are removed
// before converting the structure to a synthesized JSON schema object.
// Doing this sanitation after converting the shape to JSON schema might
// result in things like "required" properties pointing to members that
// don't exist.
Set<String> documentMemberNames = bindings.stream()
.map(HttpBinding::getMemberName)
.collect(Collectors.toSet());

// Remove non-document members.
StructureShape.Builder containerShapeBuilder = containerShape.toBuilder();
for (String memberName : containerShape.getAllMembers().keySet()) {
if (!documentMemberNames.contains(memberName)) {
containerShapeBuilder.removeMember(memberName);
}
}

StructureShape cleanedShape = containerShapeBuilder.build();
return context.getJsonSchemaConverter().convertShape(cleanedShape).getRootSchema();
}

@Override
protected Node transformSmithyValueToProtocolValue(Node value) {
return value;
}
}
Loading

0 comments on commit af61538

Please sign in to comment.