Skip to content
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
Expand Up @@ -7,14 +7,65 @@

package org.elasticsearch.xpack.inference.services.elastic;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Settings;

import java.net.URI;
import java.net.URISyntaxException;
import java.util.List;
import java.util.Objects;
import java.util.Set;

/**
* Class encapsulating any global setting for the EIS integration.
*/
public class ElasticInferenceServiceSettings {

static final Setting<String> EIS_GATEWAY_URL = Setting.simpleString("xpack.inference.eis.gateway.url", Setting.Property.NodeScope);
public static final Setting<String> EIS_GATEWAY_URL = Setting.simpleString(
"xpack.inference.eis.gateway.url",
new EisGatewayURLValidator(),
Setting.Property.NodeScope
);

private static final Logger log = LogManager.getLogger(ElasticInferenceServiceSettings.class);

/**
* Class to validate the EIS Gateway url set via `xpack.inference.eis.gateway.url`.
*/
public static class EisGatewayURLValidator implements Setting.Validator<String> {

private static final Set<String> VALID_EIS_GATEWAY_SCHEMES = Set.of("http", "https");

@Override
public void validate(String value) {
if (Objects.isNull(value) || value.isEmpty()) {
// No validation needed, if eis-gateway URL is not set
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it make sense to write to the logs that EIS is not enabled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've checked what gets logged on startup and it feels like it's rather uncommon to log something as specific as this (at least on the INFO level). There are only a few plugins logging, whether they're enabled (on a whole plugin level) like APM and security. But I think it probably doesn't cause any harm to log it on DEBUG level, so adding it with Add debug log, whether eis-gateway URL is set or not.

log.debug("eis-gateway url not set. Skipping validation");
return;
}

try {
var uri = new URI(value);
var scheme = uri.getScheme();

if (scheme == null || VALID_EIS_GATEWAY_SCHEMES.contains(scheme) == false) {
throw new IllegalArgumentException(
"["
+ scheme
+ "] is not a valid URI scheme for the setting ["
+ ElasticInferenceServiceSettings.EIS_GATEWAY_URL.getKey()
+ "]. Use one of ["
+ String.join(",", VALID_EIS_GATEWAY_SCHEMES)
+ "]"
);
}
} catch (URISyntaxException e) {
throw new IllegalArgumentException("[" + e.getInput() + "] is not a valid URI", e);
}
}
}

// Adjust this variable to be volatile, if the setting can be updated at some point in time
private final String eisGatewayUrl;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,12 @@ private URI createUri() throws URISyntaxException {
default -> throw new IllegalArgumentException("Unsupported model for EIS [" + modelId + "]");
}

return new URI(elasticInferenceServiceComponents().eisGatewayUrl() + "/sparse-text-embedding/" + modelIdUriPath);
var uriString = elasticInferenceServiceComponents().eisGatewayUrl() + "/sparse-text-embedding/" + modelIdUriPath;

// We perform the same validation here as when reading the setting to make sure that our extended URI is still valid
// This method throws, if the URI is invalid
new ElasticInferenceServiceSettings.EisGatewayURLValidator().validate(uriString);

return new URI(uriString);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

package org.elasticsearch.xpack.inference.services.elastic;

import org.elasticsearch.test.ESTestCase;

public class ElasticInferenceServiceSettingsTests extends ESTestCase {

public void testEisGatewayURLValidator_Validate_ThrowError_OnMissingURIScheme() {
expectThrows(
IllegalArgumentException.class,
() -> new ElasticInferenceServiceSettings.EisGatewayURLValidator().validate("www.missing-scheme-gateway-url.com")
);
}

public void testEisGatewayURLValidator_Validate_ThrowError_OnWrongURIScheme() {
expectThrows(
IllegalArgumentException.class,
() -> new ElasticInferenceServiceSettings.EisGatewayURLValidator().validate("file://www.missing-scheme-gateway-url.com")
);
}

public void testEisGatewayURLValidator_Validate_DoesNotThrowError_ForHTTP() {
var scheme = "http";

try {
new ElasticInferenceServiceSettings.EisGatewayURLValidator().validate(scheme + "://www.valid-gateway-url.com");
} catch (Exception e) {
fail(e, "Should not throw exception for " + "[" + scheme + "]");
}
}

public void testEisGatewayURLValidator_Validate_DoesNotThrowError_ForHTTPS() {
var scheme = "https";

try {
new ElasticInferenceServiceSettings.EisGatewayURLValidator().validate(scheme + "://www.valid-gateway-url.com");
} catch (Exception e) {
fail(e, "Should not throw exception for " + "[" + scheme + "]");
}
}

public void testEisGatewayURLValidator_Validate_DoesNotThrowError_IfURLNull() {
try {
new ElasticInferenceServiceSettings.EisGatewayURLValidator().validate(null);
} catch (Exception e) {
fail(e, "Should not throw exception for, if eis-gateway URL is null");
}
}

public void testEisGatewayURLValidator_Validate_DoesNotThrowError_IfURLEmpty() {
try {
new ElasticInferenceServiceSettings.EisGatewayURLValidator().validate("");
} catch (Exception e) {
fail(e, "Should not throw exception for, if eis-gateway URL is empty");
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,34 @@

public class ElasticInferenceServiceSparseEmbeddingsModelTests extends ESTestCase {

public void testCreateURI_ThrowError_OnMissingURIScheme() {
expectThrows(IllegalArgumentException.class, () -> createModel("www.missing-scheme-gateway-url.com"));
}

public void testCreateURI_ThrowError_OnWrongURIScheme() {
expectThrows(IllegalArgumentException.class, () -> createModel("file://www.missing-scheme-gateway-url.com"));
}

public void testCreateURI_DoesNotThrowError_ForHTTP() {
var scheme = "http";

try {
createModel(scheme + "://www.valid-gateway-url.com");
} catch (Exception e) {
fail(e, "Should not throw exception for " + "[" + scheme + "]");
}
}

public void testCreateURI_DoesNotThrowError_ForHTTPS() {
var scheme = "https";

try {
createModel(scheme + "://www.valid-gateway-url.com");
} catch (Exception e) {
fail(e, "Should not throw exception for " + "[" + scheme + "]");
}
}

public static ElasticInferenceServiceSparseEmbeddingsModel createModel(String url) {
return createModel(url, null);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ private ElasticInferenceService createServiceWithMockSender() {
return new ElasticInferenceService(
mock(HttpRequestSender.Factory.class),
createWithEmptySettings(threadPool),
new ElasticInferenceServiceComponents(null)
new ElasticInferenceServiceComponents("http://valid-eis-gateway-url.com")
);
}
}