Skip to content
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

Changed return code for RBAC and provision #1083

Merged
merged 2 commits into from
Mar 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.1.0/)

### Enhancements
### Bug Fixes
- Change REST status codes for RBAC and provisioning ([#1083](https://github.com/opensearch-project/flow-framework/pull/1083))

### Infrastructure
### Documentation
### Maintenance
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli

return channel -> client.execute(CreateWorkflowAction.INSTANCE, workflowRequest, ActionListener.wrap(response -> {
XContentBuilder builder = response.toXContent(channel.newBuilder(), ToXContent.EMPTY_PARAMS);
channel.sendResponse(new BytesRestResponse(RestStatus.CREATED, builder));
channel.sendResponse(new BytesRestResponse(provision || reprovision ? RestStatus.ACCEPTED : RestStatus.CREATED, builder));
}, exception -> {
try {
FlowFrameworkException ex = exception instanceof FlowFrameworkException
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
);
return channel -> client.execute(ProvisionWorkflowAction.INSTANCE, workflowRequest, ActionListener.wrap(response -> {
XContentBuilder builder = response.toXContent(channel.newBuilder(), ToXContent.EMPTY_PARAMS);
channel.sendResponse(new BytesRestResponse(RestStatus.OK, builder));
channel.sendResponse(new BytesRestResponse(RestStatus.ACCEPTED, builder));
}, exception -> {
try {
FlowFrameworkException ex = exception instanceof FlowFrameworkException
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -509,10 +509,7 @@ public static void onGetWorkflowResponse(
} else {
logger.debug("User: " + requestUser.getName() + " does not have permissions to access workflow: " + workflowId);
listener.onFailure(
new FlowFrameworkException(
"User does not have permissions to access workflow: " + workflowId,
RestStatus.BAD_REQUEST
)
new FlowFrameworkException("User does not have permissions to access workflow: " + workflowId, RestStatus.FORBIDDEN)
);
}
} catch (Exception e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public void testFailedUpdateWorkflow() throws Exception {
String workflowId = (String) responseMap.get(WORKFLOW_ID);

Response provisionResponse = provisionWorkflow(client(), workflowId);
assertEquals(RestStatus.OK, TestHelpers.restStatus(provisionResponse));
assertEquals(RestStatus.ACCEPTED, TestHelpers.restStatus(provisionResponse));
getAndAssertWorkflowStatus(client(), workflowId, State.PROVISIONING, ProvisioningProgress.IN_PROGRESS);

// Failed update since provisioning has started
Expand All @@ -111,7 +111,7 @@ public void testUpdateWorkflowUsingFields() throws Exception {
String workflowId = (String) responseMap.get(WORKFLOW_ID);

Response provisionResponse = provisionWorkflow(client(), workflowId);
assertEquals(RestStatus.OK, TestHelpers.restStatus(provisionResponse));
assertEquals(RestStatus.ACCEPTED, TestHelpers.restStatus(provisionResponse));
getAndAssertWorkflowStatus(client(), workflowId, State.PROVISIONING, ProvisioningProgress.IN_PROGRESS);

// Attempt to update with update_fields with illegal field
Expand Down Expand Up @@ -179,7 +179,7 @@ public void testCreateAndProvisionLocalModelWorkflow() throws Exception {

// Reattempt Provision
response = provisionWorkflow(client(), workflowId);
assertEquals(RestStatus.OK, TestHelpers.restStatus(response));
assertEquals(RestStatus.ACCEPTED, TestHelpers.restStatus(response));
getAndAssertWorkflowStatus(client(), workflowId, State.PROVISIONING, ProvisioningProgress.IN_PROGRESS);

// Wait until provisioning has completed successfully before attempting to retrieve created resources
Expand Down Expand Up @@ -240,7 +240,7 @@ public void testCreateAndProvisionRemoteModelWorkflow() throws Exception {
getAndAssertWorkflowStatus(client(), workflowId, State.NOT_STARTED, ProvisioningProgress.NOT_STARTED);

response = provisionWorkflow(client(), workflowId);
assertEquals(RestStatus.OK, TestHelpers.restStatus(response));
assertEquals(RestStatus.ACCEPTED, TestHelpers.restStatus(response));
getAndAssertWorkflowStatus(client(), workflowId, State.PROVISIONING, ProvisioningProgress.IN_PROGRESS);

// Wait until provisioning has completed successfully before attempting to retrieve created resources
Expand Down Expand Up @@ -270,7 +270,7 @@ public void testCreateAndProvisionAgentFrameworkWorkflow() throws Exception {

// Hit Create Workflow API to create agent-framework template, with template validation check and provision parameter
Response response = createWorkflowWithProvision(client(), template);
assertEquals(RestStatus.CREATED, TestHelpers.restStatus(response));
assertEquals(RestStatus.ACCEPTED, TestHelpers.restStatus(response));
Map<String, Object> responseMap = entityAsMap(response);
String workflowId = (String) responseMap.get(WORKFLOW_ID);
// wait and ensure state is completed/done
Expand Down Expand Up @@ -345,7 +345,7 @@ public void testCreateAndProvisionConnectorToolAgentFrameworkWorkflow() throws E

// Hit Create Workflow API to create agent-framework template, with template validation check and provision parameter
Response response = createWorkflowWithProvision(client(), template);
assertEquals(RestStatus.CREATED, TestHelpers.restStatus(response));
assertEquals(RestStatus.ACCEPTED, TestHelpers.restStatus(response));
Map<String, Object> responseMap = entityAsMap(response);
String workflowId = (String) responseMap.get(WORKFLOW_ID);
// wait and ensure state is completed/done
Expand Down Expand Up @@ -417,7 +417,7 @@ public void testReprovisionWorkflow() throws Exception {
Template template = TestHelpers.createTemplateFromFile("registerremotemodel.json");

Response response = createWorkflowWithProvision(client(), template);
assertEquals(RestStatus.CREATED, TestHelpers.restStatus(response));
assertEquals(RestStatus.ACCEPTED, TestHelpers.restStatus(response));
Map<String, Object> responseMap = entityAsMap(response);
String workflowId = (String) responseMap.get(WORKFLOW_ID);
// wait and ensure state is completed/done
Expand All @@ -438,7 +438,7 @@ public void testReprovisionWorkflow() throws Exception {
// Reprovision template to add ingest pipeline which uses the model ID
template = TestHelpers.createTemplateFromFile("registerremotemodel-ingestpipeline.json");
response = reprovisionWorkflow(client(), workflowId, template);
assertEquals(RestStatus.CREATED, TestHelpers.restStatus(response));
assertEquals(RestStatus.ACCEPTED, TestHelpers.restStatus(response));

resourcesCreated = getResourcesCreated(client(), workflowId, 30);
assertEquals(4, resourcesCreated.size());
Expand All @@ -457,7 +457,7 @@ public void testReprovisionWorkflow() throws Exception {
// Reprovision template to add index which uses default ingest pipeline
template = TestHelpers.createTemplateFromFile("registerremotemodel-ingestpipeline-createindex.json");
response = reprovisionWorkflow(client(), workflowId, template);
assertEquals(RestStatus.CREATED, TestHelpers.restStatus(response));
assertEquals(RestStatus.ACCEPTED, TestHelpers.restStatus(response));

resourcesCreated = getResourcesCreated(client(), workflowId, 30);
assertEquals(5, resourcesCreated.size());
Expand All @@ -475,7 +475,7 @@ public void testReprovisionWorkflow() throws Exception {
// Reprovision template to remove default ingest pipeline
template = TestHelpers.createTemplateFromFile("registerremotemodel-ingestpipeline-updateindex.json");
response = reprovisionWorkflow(client(), workflowId, template);
assertEquals(RestStatus.CREATED, TestHelpers.restStatus(response));
assertEquals(RestStatus.ACCEPTED, TestHelpers.restStatus(response));

resourcesCreated = getResourcesCreated(client(), workflowId, 30);
// resource count should remain unchanged when updating an existing node
Expand Down Expand Up @@ -504,7 +504,7 @@ public void testReprovisionWorkflowMidNodeAddition() throws Exception {
Template template = TestHelpers.createTemplateFromFile("registerremotemodel-createindex.json");

Response response = createWorkflowWithProvision(client(), template);
assertEquals(RestStatus.CREATED, TestHelpers.restStatus(response));
assertEquals(RestStatus.ACCEPTED, TestHelpers.restStatus(response));
Map<String, Object> responseMap = entityAsMap(response);
String workflowId = (String) responseMap.get(WORKFLOW_ID);
// wait and ensure state is completed/done
Expand All @@ -526,7 +526,7 @@ public void testReprovisionWorkflowMidNodeAddition() throws Exception {
// Reprovision template to add ingest pipeline which uses the model ID
template = TestHelpers.createTemplateFromFile("registerremotemodel-ingestpipeline-createindex.json");
response = reprovisionWorkflow(client(), workflowId, template);
assertEquals(RestStatus.CREATED, TestHelpers.restStatus(response));
assertEquals(RestStatus.ACCEPTED, TestHelpers.restStatus(response));

resourcesCreated = getResourcesCreated(client(), workflowId, 30);
assertEquals(5, resourcesCreated.size());
Expand Down Expand Up @@ -565,7 +565,7 @@ public void testReprovisionWithNoChange() throws Exception {
Template template = TestHelpers.createTemplateFromFile("registerremotemodel-ingestpipeline-createindex.json");

Response response = createWorkflowWithProvision(client(), template);
assertEquals(RestStatus.CREATED, TestHelpers.restStatus(response));
assertEquals(RestStatus.ACCEPTED, TestHelpers.restStatus(response));
Map<String, Object> responseMap = entityAsMap(response);
String workflowId = (String) responseMap.get(WORKFLOW_ID);
// wait and ensure state is completed/done
Expand Down Expand Up @@ -602,7 +602,7 @@ public void testReprovisionWithDeletion() throws Exception {
Template template = TestHelpers.createTemplateFromFile("registerremotemodel-ingestpipeline-createindex.json");

Response response = createWorkflowWithProvision(client(), template);
assertEquals(RestStatus.CREATED, TestHelpers.restStatus(response));
assertEquals(RestStatus.ACCEPTED, TestHelpers.restStatus(response));
Map<String, Object> responseMap = entityAsMap(response);
String workflowId = (String) responseMap.get(WORKFLOW_ID);
// wait and ensure state is completed/done
Expand Down Expand Up @@ -670,7 +670,7 @@ public void testTimestamps() throws Exception {

// Provision the template, should have created and updated same as before and provisioned newer
response = provisionWorkflow(client(), workflowId);
assertEquals(RestStatus.OK.getStatus(), response.getStatusLine().getStatusCode());
assertEquals(RestStatus.ACCEPTED.getStatus(), response.getStatusLine().getStatusCode());

response = getWorkflow(client(), workflowId);
assertEquals(RestStatus.OK.getStatus(), response.getStatusLine().getStatusCode());
Expand Down Expand Up @@ -698,7 +698,7 @@ public void testCreateAndProvisionIngestAndSearchPipeline() throws Exception {
getAndAssertWorkflowStatus(client(), workflowId, State.NOT_STARTED, ProvisioningProgress.NOT_STARTED);

response = provisionWorkflow(client(), workflowId);
assertEquals(RestStatus.OK, TestHelpers.restStatus(response));
assertEquals(RestStatus.ACCEPTED, TestHelpers.restStatus(response));
getAndAssertWorkflowStatus(client(), workflowId, State.PROVISIONING, ProvisioningProgress.IN_PROGRESS);

// Wait until provisioning has completed successfully before attempting to retrieve created resources
Expand Down Expand Up @@ -744,7 +744,7 @@ public void testDefaultCohereUseCase() throws Exception {
getAndAssertWorkflowStatus(client(), workflowId, State.NOT_STARTED, ProvisioningProgress.NOT_STARTED);

response = provisionWorkflow(client(), workflowId);
assertEquals(RestStatus.OK, TestHelpers.restStatus(response));
assertEquals(RestStatus.ACCEPTED, TestHelpers.restStatus(response));
getAndAssertWorkflowStatus(client(), workflowId, State.PROVISIONING, ProvisioningProgress.IN_PROGRESS);

// Wait until provisioning has completed successfully before attempting to retrieve created resources
Expand Down Expand Up @@ -788,7 +788,7 @@ public void testDefaultSemanticSearchUseCaseWithFailureExpected() throws Excepti
getAndAssertWorkflowStatus(client(), workflowId, State.NOT_STARTED, ProvisioningProgress.NOT_STARTED);

response = provisionWorkflow(client(), workflowId);
assertEquals(RestStatus.OK, TestHelpers.restStatus(response));
assertEquals(RestStatus.ACCEPTED, TestHelpers.restStatus(response));
getAndAssertWorkflowStatus(client(), workflowId, State.FAILED, ProvisioningProgress.FAILED);
}

Expand Down Expand Up @@ -824,7 +824,7 @@ public void testSemanticSearchWithLocalModelEndToEnd() throws Exception {
defaults.put("text_embedding.field_map.output.dimension", 384);

Response response = createAndProvisionWorkflowWithUseCaseWithContent(client(), "semantic_search_with_local_model", defaults);
assertEquals(RestStatus.CREATED, TestHelpers.restStatus(response));
assertEquals(RestStatus.ACCEPTED, TestHelpers.restStatus(response));

Map<String, Object> responseMap = entityAsMap(response);
String workflowId = (String) responseMap.get(WORKFLOW_ID);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ public void testProvisionWorkflowWithWriteAccess() throws Exception {
Map<String, Object> responseMap = entityAsMap(aliceWorkflow);
String workflowId = (String) responseMap.get(WORKFLOW_ID);
Response response = provisionWorkflow(aliceClient, workflowId);
assertEquals(RestStatus.OK, TestHelpers.restStatus(response));
assertEquals(RestStatus.ACCEPTED, TestHelpers.restStatus(response));
}

public void testReprovisionWorkflowWithWriteAccess() throws Exception {
Expand All @@ -333,7 +333,7 @@ public void testReprovisionWorkflowWithWriteAccess() throws Exception {

enableFilterBy();
Response response = createWorkflowWithProvision(aliceClient, template);
assertEquals(RestStatus.CREATED, TestHelpers.restStatus(response));
assertEquals(RestStatus.ACCEPTED, TestHelpers.restStatus(response));

Map<String, Object> responseMap = entityAsMap(response);
String workflowId = (String) responseMap.get(WORKFLOW_ID);
Expand All @@ -356,7 +356,7 @@ public void testReprovisionWorkflowWithWriteAccess() throws Exception {
// Reprovision template to add ingest pipeline which uses the model ID
template = TestHelpers.createTemplateFromFile("registerremotemodel-ingestpipeline-createindex.json");
response = reprovisionWorkflow(aliceClient, workflowId, template);
assertEquals(RestStatus.CREATED, TestHelpers.restStatus(response));
assertEquals(RestStatus.ACCEPTED, TestHelpers.restStatus(response));

resourcesCreated = getResourcesCreated(aliceClient, workflowId, 30);
assertEquals(5, resourcesCreated.size());
Expand Down Expand Up @@ -426,7 +426,7 @@ public void testCreateProvisionDeprovisionWorkflowWithFullAccess() throws Except
} else {
response = provisionWorkflow(aliceClient, workflowId);
}
assertEquals(RestStatus.OK, TestHelpers.restStatus(response));
assertEquals(RestStatus.ACCEPTED, TestHelpers.restStatus(response));

// Invoke status API
response = getWorkflowStatus(aliceClient, workflowId, false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ public void testCreateWorkflowRequestWithParamsAndProvision() throws Exception {
return null;
}).when(nodeClient).execute(any(), any(WorkflowRequest.class), any());
createWorkflowRestAction.handleRequest(request, channel, nodeClient);
assertEquals(RestStatus.CREATED, channel.capturedResponse().status());
assertEquals(RestStatus.ACCEPTED, channel.capturedResponse().status());
assertTrue(channel.capturedResponse().content().utf8ToString().contains("id-123"));
}

Expand All @@ -146,7 +146,7 @@ public void testRestCreateWorkflowWithWaitForCompletionTimeout() throws Exceptio

createWorkflowRestAction.handleRequest(request, channel, nodeClient);

assertEquals(RestStatus.CREATED, channel.capturedResponse().status());
assertEquals(RestStatus.ACCEPTED, channel.capturedResponse().status());
assertTrue(channel.capturedResponse().content().utf8ToString().contains("workflow_1"));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ public void testContentParsing() throws Exception {
return null;
}).when(nodeClient).execute(any(), any(WorkflowRequest.class), any());
provisionWorkflowRestAction.handleRequest(request, channel, nodeClient);
assertEquals(RestStatus.OK, channel.capturedResponse().status());
assertEquals(RestStatus.ACCEPTED, channel.capturedResponse().status());
assertTrue(channel.capturedResponse().content().utf8ToString().contains("id-123"));
}

Expand Down Expand Up @@ -162,7 +162,7 @@ public void testProvisionWorkflowWithValidWaitForCompletionTimeout() throws Exce

provisionWorkflowRestAction.handleRequest(request, channel, nodeClient);

assertEquals(RestStatus.OK, channel.capturedResponse().status());
assertEquals(RestStatus.ACCEPTED, channel.capturedResponse().status());
assertTrue(channel.capturedResponse().content().utf8ToString().contains("workflow_1"));
}

Expand Down
Loading
Loading