Skip to content
Open
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
3f53268
Make resource sharing records workspace-aware (spike)
DarshitChanpura Aug 4, 2026
e3467f3
Inherit resource access from workspaces on the write path
DarshitChanpura Aug 4, 2026
4672922
Guard workspace access inheritance against container cycles
DarshitChanpura Aug 5, 2026
00bd5d8
Drop version guard on workspaces serialization
DarshitChanpura Aug 6, 2026
76670dd
Batch workspace lookups on the access-check hot path
DarshitChanpura Aug 6, 2026
7412d76
Backfill workspace membership during migration
DarshitChanpura Aug 8, 2026
d71f36d
Merge remote-tracking branch 'upstream/main' into workspace-aware-sha…
DarshitChanpura Aug 8, 2026
bcc3024
Address Code-Diff-Analyzer findings on workspace sharing
DarshitChanpura Aug 8, 2026
0a7486f
Merge remote-tracking branch 'upstream/main' into workspace-aware-sha…
DarshitChanpura Aug 25, 2026
5333eba
Make ResourceSharing round-trip over the transport wire
DarshitChanpura Aug 25, 2026
19af1df
Backfill workspaces onto already-migrated sharing records
DarshitChanpura Aug 25, 2026
3115d4c
Integration-test workspace backfill during migration
DarshitChanpura Aug 25, 2026
48ccf99
Integration-test workspace-aware live indexing
DarshitChanpura Aug 26, 2026
533c69a
Add SPI seam for trusted workspace-membership resolution
DarshitChanpura Aug 26, 2026
31fde47
Unit-test workspace index-handler + extraction paths
DarshitChanpura Aug 26, 2026
114fdc4
Filter workspace visibility on the resource workspaces field
DarshitChanpura Sep 8, 2026
8db592c
Merge remote-tracking branch 'upstream/main' into workspace-aware-sha…
DarshitChanpura Sep 8, 2026
a1dc646
Filter DLS on the provider-declared workspaces field
DarshitChanpura Sep 9, 2026
e2f4225
Simplify workspace access checks; address review nits
DarshitChanpura Sep 9, 2026
d86d9c6
Make workspace-sharing comments crisp and self-contained
DarshitChanpura Sep 9, 2026
725dfb3
Keep sharing records in sync with resource workspaces
DarshitChanpura Sep 10, 2026
361ec6b
Register workspace container type in sample plugin
DarshitChanpura Sep 10, 2026
1577069
Expect workspace as a shareable type in sample types API
DarshitChanpura Sep 10, 2026
e82c018
Make workspace reconciliation monotonic and workspace field trusted
DarshitChanpura Sep 10, 2026
eca8bdf
Merge remote-tracking branch 'upstream/main' into workspace-aware-sha…
DarshitChanpura Sep 10, 2026
7c5cf51
Make workspaces guard durable across record rewrites
DarshitChanpura Sep 10, 2026
81f704c
Guard concurrent share/patch against reconcile clobber
DarshitChanpura Sep 10, 2026
8f62005
Restore context on fetch failure; preserve parse cause; strengthen CA…
DarshitChanpura Sep 10, 2026
649432f
Tighten workspace javadoc and inline comments
DarshitChanpura Sep 10, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ public void testBehaviorAfterEnabling() throws Exception {
migrateResponse.assertStatusCode(HttpStatus.SC_OK);
assertThat(
migrateResponse.bodyAsMap().get("summary"),
equalTo("Migration complete. migrated 1; skippedNoType 0; skippedExisting 0; failed 0")
equalTo("Migration complete. migrated 1; backfilledExisting 0; skippedNoType 0; skippedExisting 0; failed 0")
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ public void testResourceProtected() throws Exception {
migrateResponse.assertStatusCode(HttpStatus.SC_OK);
assertThat(
migrateResponse.bodyAsMap().get("summary"),
equalTo("Migration complete. migrated 1; skippedNoType 0; skippedExisting 0; failed 0")
equalTo("Migration complete. migrated 1; backfilledExisting 0; skippedNoType 0; skippedExisting 0; failed 0")
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ public void testMigrateAPIWithRestAdmin_valid() {
migrateResponse.assertStatusCode(HttpStatus.SC_OK);
assertThat(
migrateResponse.bodyAsMap().get("summary"),
equalTo("Migration complete. migrated 2; skippedNoType 0; skippedExisting 0; failed 0")
equalTo("Migration complete. migrated 2; backfilledExisting 0; skippedNoType 0; skippedExisting 0; failed 0")
);
assertThat(migrateResponse.bodyAsMap().get("resourcesWithDefaultOwner"), equalTo(List.of(resourceIdNoUser)));
}
Expand Down Expand Up @@ -215,7 +215,7 @@ public void testMigrateAPIWithSuperAdmin_valid() {
migrateResponse.assertStatusCode(HttpStatus.SC_OK);
assertThat(
migrateResponse.bodyAsMap().get("summary"),
equalTo("Migration complete. migrated 2; skippedNoType 0; skippedExisting 0; failed 0")
equalTo("Migration complete. migrated 2; backfilledExisting 0; skippedNoType 0; skippedExisting 0; failed 0")
);
assertThat(migrateResponse.bodyAsMap().get("resourcesWithDefaultOwner"), equalTo(List.of(resourceIdNoUser)));

Expand Down Expand Up @@ -247,7 +247,7 @@ public void testMigrateTwice_shouldSkipSecondTime() {
migrateResponse.assertStatusCode(HttpStatus.SC_OK);
assertThat(
migrateResponse.bodyAsMap().get("summary"),
equalTo("Migration complete. migrated 2; skippedNoType 0; skippedExisting 0; failed 0")
equalTo("Migration complete. migrated 2; backfilledExisting 0; skippedNoType 0; skippedExisting 0; failed 0")
);
assertThat(migrateResponse.bodyAsMap().get("resourcesWithDefaultOwner"), equalTo(List.of(resourceIdNoUser)));

Expand All @@ -273,7 +273,7 @@ public void testMigrateTwice_shouldSkipSecondTime() {
migrateResponse.assertStatusCode(HttpStatus.SC_OK);
assertThat(
migrateResponse.bodyAsMap().get("summary"),
equalTo("Migration complete. migrated 0; skippedNoType 0; skippedExisting 2; failed 0")
equalTo("Migration complete. migrated 0; backfilledExisting 0; skippedNoType 0; skippedExisting 2; failed 0")
);
assertThat(migrateResponse.bodyAsMap().get("resourcesWithDefaultOwner"), equalTo(List.of(resourceIdNoUser)));

Expand All @@ -294,6 +294,91 @@ public void testMigrateTwice_shouldSkipSecondTime() {
}
}

@Test
public void testLiveIndexingStampsWorkspacesOnSharingRecord() {
// Steady-state: creating a resource with a workspaces field must trigger ResourceIndexListener to
// extract the (multi-valued) workspaces from the parsed doc via extractMultiValuedFieldFromIndexOp and
// store them on the sharing record (used by the write-path access-level fan-out). Workspace membership is
Comment thread
DarshitChanpura marked this conversation as resolved.
Outdated
// NOT projected into all_shared_principals; read-path visibility filters the resource's own `workspaces`
// field in DLS. Also confirms the Lucene getFields() materialization works for the sample plugin's mapping.
String resourceId = createSampleResourceWithWorkspaces("ws-a", "ws-b");

try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) {
// The sharing record carries the workspaces field (for the write-path fan-out).
TestRestClient.HttpResponse sharingDoc = client.get(RESOURCE_SHARING_INDEX + "/_doc/" + resourceId);
sharingDoc.assertStatusCode(HttpStatus.SC_OK);
ArrayNode ws = (ArrayNode) sharingDoc.bodyAsJsonNode().get("_source").get("workspaces");
List<String> workspaceIds = new ArrayList<>();
ws.forEach(n -> workspaceIds.add(n.asString()));
assertThat(workspaceIds, containsInAnyOrder("ws-a", "ws-b"));

// all_shared_principals stays usernames/roles only -- no workspace:<id> denormalization.
TestRestClient.HttpResponse resourceDoc = client.get(RESOURCE_INDEX_NAME + "/_doc/" + resourceId);
resourceDoc.assertStatusCode(HttpStatus.SC_OK);
ArrayNode principals = (ArrayNode) resourceDoc.bodyAsJsonNode().get("_source").get("all_shared_principals");
List<String> principalList = new ArrayList<>();
principals.forEach(n -> principalList.add(n.asString()));
assertThat(principalList, containsInAnyOrder("user:" + MIGRATION_USER.getName()));

// The resource doc keeps its own `workspaces` field -- this is what DLS filters on for read visibility.
ArrayNode docWs = (ArrayNode) resourceDoc.bodyAsJsonNode().get("_source").get("workspaces");
List<String> docWorkspaceIds = new ArrayList<>();
docWs.forEach(n -> docWorkspaceIds.add(n.asString()));
assertThat(docWorkspaceIds, containsInAnyOrder("ws-a", "ws-b"));
}
}

@Test
public void testMigrateBackfillsWorkspacesOntoExistingRecord() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If I'm reading this correctly, the migrate API can now be called multiple times? One from a version before this PR and one afterwards to get the workspace info?

// A resource whose sharing record already exists (created at resource-creation time) but which has
// since gained workspace membership on its source doc. Migration should not re-create the record; it
// should backfill the workspaces field and refresh all_shared_principals.
String resourceId = createSampleResource();

try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) {
// Add workspace membership to the resource's source doc (an _update, so no new sharing record is
// created). The existing sharing record stays workspace-blind until migration backfills it.
TestRestClient.HttpResponse update = client.postJson(
RESOURCE_INDEX_NAME + "/_update/" + resourceId + "?refresh=true",
"{ \"doc\": { \"workspaces\": [\"ws-a\", \"ws-b\"] } }"
);
update.assertStatusCode(HttpStatus.SC_OK);

// Migrate without clearing: the record exists, so create is skipped; the source doc now has
// workspaces, so it is backfilled rather than skipped.
TestRestClient.HttpResponse migrateResponse = client.postJson(RESOURCE_SHARING_MIGRATION_ENDPOINT, migrationPayload_valid());
migrateResponse.assertStatusCode(HttpStatus.SC_OK);
assertThat(
migrateResponse.bodyAsMap().get("summary"),
equalTo("Migration complete. migrated 0; backfilledExisting 1; skippedNoType 0; skippedExisting 0; failed 0")
);

// The sharing record now carries the workspaces field.
TestRestClient.HttpResponse sharingDoc = client.get(RESOURCE_SHARING_INDEX + "/_doc/" + resourceId);
sharingDoc.assertStatusCode(HttpStatus.SC_OK);
ArrayNode ws = (ArrayNode) sharingDoc.bodyAsJsonNode().get("_source").get("workspaces");
List<String> workspaceIds = new ArrayList<>();
ws.forEach(n -> workspaceIds.add(n.asString()));
assertThat(workspaceIds, containsInAnyOrder("ws-a", "ws-b"));

// all_shared_principals stays usernames/roles only -- workspace membership is not denormalized here.
TestRestClient.HttpResponse resourceDoc = client.get(RESOURCE_INDEX_NAME + "/_doc/" + resourceId);
resourceDoc.assertStatusCode(HttpStatus.SC_OK);
ArrayNode principals = (ArrayNode) resourceDoc.bodyAsJsonNode().get("_source").get("all_shared_principals");
List<String> principalList = new ArrayList<>();
principals.forEach(n -> principalList.add(n.asString()));
assertThat(principalList, containsInAnyOrder("user:" + MIGRATION_USER.getName()));

// Idempotency: a second migrate with the same workspaces adds nothing new (skipped, not backfilled).
TestRestClient.HttpResponse secondMigrate = client.postJson(RESOURCE_SHARING_MIGRATION_ENDPOINT, migrationPayload_valid());
secondMigrate.assertStatusCode(HttpStatus.SC_OK);
assertThat(
secondMigrate.bodyAsMap().get("summary"),
equalTo("Migration complete. migrated 0; backfilledExisting 0; skippedNoType 0; skippedExisting 1; failed 0")
);
}
}

@Test
public void testMigrateAPIWithSuperAdmin_valid_withSpecifiedAccessLevel() {
String resourceId = createSampleResource();
Expand All @@ -308,7 +393,7 @@ public void testMigrateAPIWithSuperAdmin_valid_withSpecifiedAccessLevel() {
migrateResponse.assertStatusCode(HttpStatus.SC_OK);
assertThat(
migrateResponse.bodyAsMap().get("summary"),
equalTo("Migration complete. migrated 2; skippedNoType 0; skippedExisting 0; failed 0")
equalTo("Migration complete. migrated 2; backfilledExisting 0; skippedNoType 0; skippedExisting 0; failed 0")
);
assertThat(migrateResponse.bodyAsMap().get("resourcesWithDefaultOwner"), equalTo(List.of(resourceIdNoUser)));

Expand Down Expand Up @@ -395,7 +480,7 @@ public void testMigrateAPIWithSuperAdmin_noDefaultAccessLevel_usesRegisteredDefa
migrateResponse.assertStatusCode(HttpStatus.SC_OK);
assertThat(
migrateResponse.bodyAsMap().get("summary"),
equalTo("Migration complete. migrated 2; skippedNoType 0; skippedExisting 0; failed 0")
equalTo("Migration complete. migrated 2; backfilledExisting 0; skippedNoType 0; skippedExisting 0; failed 0")
);

TestRestClient.HttpResponse sharingResponse = client.get(RESOURCE_SHARING_INDEX + "/_search");
Expand Down Expand Up @@ -645,7 +730,7 @@ public void testMigrateAPI_withGarbageParentId() {
migrateResponse.assertStatusCode(HttpStatus.SC_OK);
assertThat(
migrateResponse.bodyAsMap().get("summary"),
equalTo("Migration complete. migrated 1; skippedNoType 0; skippedExisting 0; failed 0")
equalTo("Migration complete. migrated 1; backfilledExisting 0; skippedNoType 0; skippedExisting 0; failed 0")
);

// The sharing record should be created with the garbage parent_id stored as-is
Expand Down Expand Up @@ -681,7 +766,7 @@ public void testMigrateAPI_withParentHierarchy() {
migrateResponse.assertStatusCode(HttpStatus.SC_OK);
assertThat(
migrateResponse.bodyAsMap().get("summary"),
equalTo("Migration complete. migrated 2; skippedNoType 0; skippedExisting 0; failed 0")
equalTo("Migration complete. migrated 2; backfilledExisting 0; skippedNoType 0; skippedExisting 0; failed 0")
);

// Verify the sharing record for the resource has parent_type and parent_id set
Expand Down Expand Up @@ -762,6 +847,26 @@ private String createSampleResource() {
}
}

private String createSampleResourceWithWorkspaces(String... workspaceIds) {
try (TestRestClient client = cluster.getRestClient(MIGRATION_USER)) {
StringBuilder wsArray = new StringBuilder("[");
for (int i = 0; i < workspaceIds.length; i++) {
if (i > 0) wsArray.append(",");
wsArray.append("\"").append(workspaceIds[i]).append("\"");
}
wsArray.append("]");
String sampleResource = ("{\"name\":\"sample_ws\",\"store_user\":true,\"workspaces\":" + wsArray + "}");

TestRestClient.HttpResponse response = client.putJson(SAMPLE_RESOURCE_CREATE_ENDPOINT, sampleResource);
response.assertStatusCode(HttpStatus.SC_OK);
String resourceId = response.getTextFromJsonBody("/message").split(":")[1].trim();
Awaitility.await()
.alias("Wait until resource with workspaces is populated")
.until(() -> client.get(SAMPLE_RESOURCE_GET_ENDPOINT + "/" + resourceId).getStatusCode(), equalTo(200));
return resourceId;
}
}

private String createSampleResourceNoUser() {
try (TestRestClient client = cluster.getRestClient(MIGRATION_USER)) {
String sampleResource = """
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@
package org.opensearch.sample;

import java.io.IOException;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;

import org.opensearch.commons.authuser.User;
import org.opensearch.core.ParseField;
Expand Down Expand Up @@ -40,6 +43,9 @@ public class SampleResource implements NamedWriteable, ToXContentObject {
private Map<String, String> attributes;
// NOTE: following field is added to specifically test migrate API, for newer resources this field must not be defined
private User user;
// Workspace membership; optional, models the multi-valued "workspaces" field a real workspace-aware resource
// would declare so ResourceIndexListener can project workspace:<id> into all_shared_principals.
private Set<String> workspaces;

public SampleResource() throws IOException {
super();
Expand All @@ -51,6 +57,8 @@ public SampleResource(StreamInput in) throws IOException {
this.groupId = in.readOptionalString();
this.attributes = in.readMap(StreamInput::readString, StreamInput::readString);
this.user = new User(in);
List<String> ws = in.readOptionalStringList();
this.workspaces = ws == null ? null : new HashSet<>(ws);
}

private static final ConstructingObjectParser<SampleResource, Void> PARSER = new ConstructingObjectParser<>(RESOURCE_TYPE, true, a -> {
Expand All @@ -67,6 +75,10 @@ public SampleResource(StreamInput in) throws IOException {
// ignore a[3] as we know the type
s.setAttributes((Map<String, String>) a[4]);
s.setUser((User) a[5]);
List<String> ws = (List<String>) a[6];
if (ws != null) {
s.setWorkspaces(new HashSet<>(ws));
}
return s;
});

Expand All @@ -77,21 +89,27 @@ public SampleResource(StreamInput in) throws IOException {
PARSER.declareStringOrNull(optionalConstructorArg(), new ParseField("resource_type"));
PARSER.declareObjectOrNull(optionalConstructorArg(), (p, c) -> p.mapStrings(), null, new ParseField("attributes"));
PARSER.declareObjectOrNull(optionalConstructorArg(), (p, c) -> User.parse(p), null, new ParseField("user"));
PARSER.declareStringArray(optionalConstructorArg(), new ParseField("workspaces"));
}

public static SampleResource fromXContent(XContentParser parser) throws IOException {
return PARSER.parse(parser, null);
}

public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params params) throws IOException {
return builder.startObject()
builder.startObject()
.field("name", name)
.field("description", description)
.field("group_id", groupId)
.field("resource_type", RESOURCE_TYPE)
.field("attributes", attributes)
.field("user", user)
.endObject();
.field("user", user);
// Emit workspaces only when non-empty so pre-existing docs stay byte-identical (BWC for callers/tests
// that don't touch this field).
if (workspaces != null && !workspaces.isEmpty()) {
builder.field("workspaces", workspaces);
}
return builder.endObject();
}

public void writeTo(StreamOutput out) throws IOException {
Expand All @@ -100,6 +118,8 @@ public void writeTo(StreamOutput out) throws IOException {
out.writeOptionalString(groupId);
out.writeMap(attributes, StreamOutput::writeString, StreamOutput::writeString);
user.writeTo(out);
// Symmetric with the StreamInput ctor. Passing null when unset keeps mixed-caller compatibility.
out.writeOptionalStringCollection(workspaces);
}

public void setName(String name) {
Expand All @@ -122,6 +142,14 @@ public void setUser(User user) {
this.user = user;
}

public void setWorkspaces(Set<String> workspaces) {
this.workspaces = workspaces;
}

public Set<String> getWorkspaces() {
return workspaces;
}

public String getName() {
return name;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

package org.opensearch.sample;

import java.util.Collections;
import java.util.HashSet;
import java.util.Set;

import org.opensearch.sample.client.ResourceSharingClientAccessor;
Expand Down Expand Up @@ -54,11 +56,36 @@ public String parentType() {
public String parentIdField() {
return "group_id";
}

@Override
public String workspacesField() {
return "workspaces";
}
});
}

@Override
public void assignResourceSharingClient(ResourceSharingClient resourceSharingClient) {
ResourceSharingClientAccessor.getInstance().setResourceSharingClient(resourceSharingClient);
}

/**
* Test-only workspace-membership resolver. Maps a user's <em>security roles</em> to a deterministic workspace ID
* ({@code ws-<role>}), simulating a trusted server-set source. Roles are resolved by the security plugin at
* authc time, so they are not user-assertable — matching the SPI contract.
*
* <p>A real workspace-owning plugin would replace this with a lookup against its own authoritative store
* (populated at authc time or cached in memory), never with values derived from user-influenceable inputs.
*/
@Override
public Set<String> resolveWorkspacesForUser(String username, Set<String> securityRoles, Set<String> backendRoles) {
if (securityRoles == null || securityRoles.isEmpty()) {
return Collections.emptySet();
}
Set<String> workspaces = new HashSet<>();
for (String role : securityRoles) {
workspaces.add("ws-" + role);
}
return workspaces;
}
}
Loading
Loading