-
Notifications
You must be signed in to change notification settings - Fork 183
Description
What is the bug?
In #3805, the validate()
method of the MLCreateConnectorRequest
was used to add field validation using StringUtils
.
Unfortunately, validate()
is called from the TransportActionNodeProxy
class with an org.opensearch.action.Request
class, and the StringUtils
class is not available to the classloader used for this call.
How can one reproduce the bug?
Steps to reproduce the behavior:
- Attempt to create a connector using the ML Client's
createConnector()
method from another plugin (for example, any Flow Framework plugin integ test that creates a connector). - Observe the request failing silently with an exception (probably class loading?) swallowed silently somewhere.
- Add debug logging to see that
validate()
was working fine but the call tovalidateFields()
was never completing successfully. (Was never able to reproduce an exception on the logging thread.)
What is the expected behavior?
A connector is created or field validation fails with a 400 error.
What is your host/environment?
Easy to reproduce locally on macOS running Flow Framework integ tests on current main
branch beginning with the commit including #3805.
Also has caused integ test failures on all platforms on GitHub Action integ tests as well as Jenkins test environment, see opensearch-project/flow-framework#1143
Do you have any additional context?
I've worked around the problem locally by separating the field validation code from the validate()
method.
@Override
public ActionRequestValidationException validate() {
// This is called with a Request class and a different classloader so must be simple
if (mlCreateConnectorInput == null) {
return addValidationError("ML Connector input can't be null", null);
}
return null;
}
// This must be called from a thread with access to this class's packages
public static ActionRequestValidationException validateFields(MLCreateConnectorInput mlCreateConnectorInput) {
Map<String, FieldDescriptor> fieldsToValidate = new HashMap<>();
fieldsToValidate.put("Model connector name", new FieldDescriptor(mlCreateConnectorInput.getName(), true));
fieldsToValidate.put("Model connector description", new FieldDescriptor(mlCreateConnectorInput.getDescription(), false));
return StringUtils.validateFields(fieldsToValidate);
}
And then, from TransportCreateConnectorAction
:
@Override
protected void doExecute(Task task, ActionRequest request, ActionListener<MLCreateConnectorResponse> listener) {
MLCreateConnectorRequest mlCreateConnectorRequest = MLCreateConnectorRequest.fromActionRequest(request);
MLCreateConnectorInput mlCreateConnectorInput = mlCreateConnectorRequest.getMlCreateConnectorInput();
// existing protocol, tenant, and dryrun conditionals
String connectorName = mlCreateConnectorInput.getName();
try {
// add this line
MLCreateConnectorRequest.validateFields(mlCreateConnectorInput);
// remainder of the class
Whether to use a static or instance method or validate as part of fromActionRequest()
is left as a style preference; the goal here was to identify a workaround, not the chosen solution. It may be useful to put the validation in the MLCreateConnectorInput
class rather than here.
The same pattern may be necessary for other Transport Actions updated in #3805 (update connector, register/update model and model group), as validate()
will suffer the same shortcoming there.
Metadata
Metadata
Assignees
Labels
Type
Projects
Status