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

Implement datasource update API #292

Merged
merged 2 commits into from
May 10, 2023
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.geospatial.ip2geo.action;

import static org.opensearch.geospatial.shared.URLBuilder.URL_DELIMITER;
import static org.opensearch.geospatial.shared.URLBuilder.getPluginURLPrefix;
import static org.opensearch.rest.RestRequest.Method.PUT;

import java.io.IOException;
import java.util.List;

import org.opensearch.client.node.NodeClient;
import org.opensearch.core.xcontent.XContentParser;
import org.opensearch.rest.BaseRestHandler;
import org.opensearch.rest.RestRequest;
import org.opensearch.rest.action.RestToXContentListener;

/**
* Rest handler for Ip2Geo datasource update request
*/
public class RestUpdateDatasourceHandler extends BaseRestHandler {
private static final String ACTION_NAME = "ip2geo_datasource_update";

@Override
public String getName() {
return ACTION_NAME;
}

@Override
protected RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
final PutDatasourceRequest putDatasourceRequest = new PutDatasourceRequest(request.param("name"));
if (request.hasContentOrSourceParam()) {
heemin32 marked this conversation as resolved.
Show resolved Hide resolved
try (XContentParser parser = request.contentOrSourceParamParser()) {
PutDatasourceRequest.PARSER.parse(parser, putDatasourceRequest, null);
}
}
return channel -> client.executeLocally(
UpdateDatasourceAction.INSTANCE,
putDatasourceRequest,
new RestToXContentListener<>(channel)
);
}

@Override
public List<Route> routes() {
String path = String.join(URL_DELIMITER, getPluginURLPrefix(), "ip2geo/datasource/{name}/_settings");
return List.of(new Route(PUT, path));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.geospatial.ip2geo.action;

import org.opensearch.action.ActionType;
import org.opensearch.action.support.master.AcknowledgedResponse;

/**
* Ip2Geo datasource update action
*/
public class UpdateDatasourceAction extends ActionType<AcknowledgedResponse> {
/**
* Update datasource action instance
*/
public static final UpdateDatasourceAction INSTANCE = new UpdateDatasourceAction();
/**
* Update datasource action name
*/
public static final String NAME = "cluster:admin/geospatial/datasource/update";

private UpdateDatasourceAction() {
super(NAME, AcknowledgedResponse::new);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.geospatial.ip2geo.action;

import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URISyntaxException;
import java.net.URL;
import java.util.Locale;

import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.Setter;
import lombok.extern.log4j.Log4j2;

import org.opensearch.action.ActionRequestValidationException;
import org.opensearch.action.support.master.AcknowledgedRequest;
import org.opensearch.common.io.stream.StreamInput;
import org.opensearch.common.io.stream.StreamOutput;
import org.opensearch.common.unit.TimeValue;
import org.opensearch.core.ParseField;
import org.opensearch.core.xcontent.ObjectParser;
import org.opensearch.geospatial.ip2geo.common.DatasourceManifest;

/**
* Ip2Geo datasource update request
*/
@Getter
@Setter
@Log4j2
@EqualsAndHashCode(callSuper = false)
public class UpdateDatasourceRequest extends AcknowledgedRequest<UpdateDatasourceRequest> {
private static final ParseField ENDPOINT_FIELD = new ParseField("endpoint");
private static final ParseField UPDATE_INTERVAL_IN_DAYS_FIELD = new ParseField("update_interval_in_days");
private static final int MAX_DATASOURCE_NAME_BYTES = 255;
/**
* @param name the datasource name
* @return the datasource name
*/
private String name;
/**
* @param endpoint url to a manifest file for a datasource
* @return url to a manifest file for a datasource
*/
private String endpoint;
/**
* @param updateInterval update interval of a datasource
* @return update interval of a datasource
*/
private TimeValue updateInterval;

/**
* Parser of a datasource
*/
public static final ObjectParser<UpdateDatasourceRequest, Void> PARSER;
static {
PARSER = new ObjectParser<>("update_datasource");
PARSER.declareString((request, val) -> request.setEndpoint(val), ENDPOINT_FIELD);
PARSER.declareLong((request, val) -> request.setUpdateInterval(TimeValue.timeValueDays(val)), UPDATE_INTERVAL_IN_DAYS_FIELD);
}

/**
* Constructor
* @param name name of a datasource
*/
public UpdateDatasourceRequest(final String name) {
Copy link
Member

Choose a reason for hiding this comment

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

Don't you need endpoint?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Could you elaborate more? Endpoint is not needed here.

this.name = name;
}

/**
* Constructor
* @param in the stream input
* @throws IOException IOException
*/
public UpdateDatasourceRequest(final StreamInput in) throws IOException {
super(in);
this.name = in.readString();
this.endpoint = in.readOptionalString();
this.updateInterval = in.readOptionalTimeValue();
}

@Override
public void writeTo(final StreamOutput out) throws IOException {
super.writeTo(out);
out.writeString(name);
out.writeOptionalString(endpoint);
out.writeOptionalTimeValue(updateInterval);
}

@Override
public ActionRequestValidationException validate() {
ActionRequestValidationException errors = new ActionRequestValidationException();
if (endpoint == null && updateInterval == null) {
errors.addValidationError("no values to update");
}

validateEndpoint(errors);
validateUpdateInterval(errors);

return errors.validationErrors().isEmpty() ? null : errors;
}

/**
* Conduct following validation on endpoint
* 1. endpoint format complies with RFC-2396
* 2. validate manifest file from the endpoint
*
* @param errors the errors to add error messages
*/
private void validateEndpoint(final ActionRequestValidationException errors) {
if (endpoint == null) {
return;
}

try {
URL url = new URL(endpoint);
url.toURI(); // Validate URL complies with RFC-2396
validateManifestFile(url, errors);
} catch (MalformedURLException | URISyntaxException e) {
log.info("Invalid URL[{}] is provided", endpoint, e);
errors.addValidationError("Invalid URL format is provided");
Copy link
Member

Choose a reason for hiding this comment

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

can you add url to validation error message?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is invalid URL. So, we don't want to return the url back to user which could cause a security risk.

}
}

/**
* Conduct following validation on url
* 1. can read manifest file from the endpoint
* 2. the url in the manifest file complies with RFC-2396
*
* @param url the url to validate
* @param errors the errors to add error messages
*/
private void validateManifestFile(final URL url, final ActionRequestValidationException errors) {
DatasourceManifest manifest;
try {
manifest = DatasourceManifest.Builder.build(url);
} catch (Exception e) {
log.info("Error occurred while reading a file from {}", url, e);
errors.addValidationError(String.format(Locale.ROOT, "Error occurred while reading a file from %s: %s", url, e.getMessage()));
Copy link
Member

Choose a reason for hiding this comment

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

is it safe to include customer provided URL to error message here?

Copy link
Collaborator Author

@heemin32 heemin32 May 8, 2023

Choose a reason for hiding this comment

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

Yes. Because this URL is already validated as having correct format. Only failed to read file from it.

return;
}

try {
new URL(manifest.getUrl()).toURI(); // Validate URL complies with RFC-2396
} catch (MalformedURLException | URISyntaxException e) {
log.info("Invalid URL[{}] is provided for url field in the manifest file", manifest.getUrl(), e);
errors.addValidationError("Invalid URL format is provided for url field in the manifest file");
}
}

/**
* Validate updateInterval is equal or larger than 1
*
* @param errors the errors to add error messages
*/
private void validateUpdateInterval(final ActionRequestValidationException errors) {
if (updateInterval == null) {
return;
Comment on lines +160 to +161
Copy link
Member

Choose a reason for hiding this comment

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

is null update interval valid?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If the value is null, we don't update the value. As long as one of values are not null, the request is valid.

}

if (updateInterval.compareTo(TimeValue.timeValueDays(1)) < 0) {
errors.addValidationError("Update interval should be equal to or larger than 1 day");
}
}
}
Loading