-
Notifications
You must be signed in to change notification settings - Fork 25.6k
[ML] Propagate auth token through EIS logic #137902
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
Changes from all commits
35539ea
89177b1
fe45947
3c5555e
f82bc24
6a40132
19a45e8
d0c2962
c4dbe2a
6f4cf9f
e53d666
8768b33
482b09b
0bcedbd
821b947
feca486
484a648
4451505
5a3553e
76335fc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,7 @@ | |
| import org.elasticsearch.xpack.inference.services.elastic.InternalPreconfiguredEndpoints; | ||
| import org.elasticsearch.xpack.inference.services.elastic.authorization.AuthorizationPoller; | ||
| import org.elasticsearch.xpack.inference.services.elastic.authorization.AuthorizationTaskExecutor; | ||
| import org.elasticsearch.xpack.inference.services.elastic.ccm.CCMSettings; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Most of the changes in this class were to make the functions available to other classes by making them static and default scope. |
||
| import org.junit.After; | ||
| import org.junit.AfterClass; | ||
| import org.junit.Before; | ||
|
|
@@ -87,6 +88,10 @@ public void createComponents() { | |
|
|
||
| @After | ||
| public void shutdown() { | ||
| removeEisPreconfiguredEndpoints(modelRegistry); | ||
| } | ||
|
|
||
| static void removeEisPreconfiguredEndpoints(ModelRegistry modelRegistry) { | ||
| // Delete all the eis preconfigured endpoints | ||
| var listener = new PlainActionFuture<Boolean>(); | ||
| modelRegistry.deleteModels(InternalPreconfiguredEndpoints.EIS_PRECONFIGURED_ENDPOINT_IDS, listener); | ||
|
|
@@ -101,6 +106,8 @@ public static void cleanUpClass() { | |
| @Override | ||
| protected Settings nodeSettings() { | ||
| return Settings.builder() | ||
| // Disable CCM to ensure that only the authorization task executor is initialized in the inference plugin when it is created | ||
| .put(CCMSettings.CCM_SUPPORTED_ENVIRONMENT.getKey(), false) | ||
| .put(ElasticInferenceServiceSettings.ELASTIC_INFERENCE_SERVICE_URL.getKey(), gatewayUrl) | ||
| // Ensure that the polling logic only occurs once so we can deterministically control when an authorization response is | ||
| // received | ||
|
|
@@ -123,15 +130,23 @@ public void testCreatesEisChatCompletionEndpoint() throws Exception { | |
| } | ||
|
|
||
| private void assertNoAuthorizedEisEndpoints() throws Exception { | ||
| waitForTask(AUTH_TASK_ACTION, admin()); | ||
| assertNoAuthorizedEisEndpoints(admin(), authorizationTaskExecutor, modelRegistry); | ||
| } | ||
|
|
||
| static void assertNoAuthorizedEisEndpoints( | ||
| AdminClient adminClient, | ||
| AuthorizationTaskExecutor authorizationTaskExecutor, | ||
| ModelRegistry modelRegistry | ||
| ) throws Exception { | ||
| waitForTask(AUTH_TASK_ACTION, adminClient); | ||
|
|
||
| assertBusy(() -> { | ||
| var newPoller = authorizationTaskExecutor.getCurrentPollerTask(); | ||
| assertNotNull(newPoller); | ||
| newPoller.waitForAuthorizationToComplete(TimeValue.THIRTY_SECONDS); | ||
| }); | ||
|
|
||
| var eisEndpoints = getEisEndpoints(); | ||
| var eisEndpoints = getEisEndpoints(modelRegistry); | ||
| assertThat(eisEndpoints, empty()); | ||
|
|
||
| for (String eisPreconfiguredEndpoints : InternalPreconfiguredEndpoints.EIS_PRECONFIGURED_ENDPOINT_IDS) { | ||
|
|
@@ -153,7 +168,22 @@ public static TaskInfo waitForTask(String taskAction, AdminClient adminClient) t | |
| return taskRef.get(); | ||
| } | ||
|
|
||
| static void waitForNoTask(String taskAction, AdminClient adminClient) throws Exception { | ||
| var builder = new ListTasksRequestBuilder(adminClient.cluster()); | ||
|
|
||
| assertBusy(() -> { | ||
| var response = builder.get(); | ||
| var authPollerTask = response.getTasks().stream().filter(task -> task.action().equals(taskAction)).findFirst(); | ||
| assertFalse(authPollerTask.isPresent()); | ||
| }); | ||
|
|
||
| } | ||
|
|
||
| private List<UnparsedModel> getEisEndpoints() { | ||
| return getEisEndpoints(modelRegistry); | ||
| } | ||
|
|
||
| static List<UnparsedModel> getEisEndpoints(ModelRegistry modelRegistry) { | ||
| var listener = new PlainActionFuture<List<UnparsedModel>>(); | ||
| modelRegistry.getAllModels(false, listener); | ||
|
|
||
|
|
@@ -162,17 +192,26 @@ private List<UnparsedModel> getEisEndpoints() { | |
| } | ||
|
|
||
| private void restartPollingTaskAndWaitForAuthResponse() throws Exception { | ||
| cancelAuthorizationTask(admin()); | ||
| restartPollingTaskAndWaitForAuthResponse(admin(), authorizationTaskExecutor); | ||
| } | ||
|
|
||
| static void restartPollingTaskAndWaitForAuthResponse(AdminClient adminClient, AuthorizationTaskExecutor authTaskExecutor) | ||
| throws Exception { | ||
| cancelAuthorizationTask(adminClient); | ||
|
|
||
| // wait for the new task to be recreated and an authorization response to be processed | ||
| waitForAuthorizationToComplete(authTaskExecutor); | ||
| } | ||
|
|
||
| static void waitForAuthorizationToComplete(AuthorizationTaskExecutor authTaskExecutor) throws Exception { | ||
| assertBusy(() -> { | ||
| var newPoller = authorizationTaskExecutor.getCurrentPollerTask(); | ||
| var newPoller = authTaskExecutor.getCurrentPollerTask(); | ||
| assertNotNull(newPoller); | ||
| newPoller.waitForAuthorizationToComplete(TimeValue.THIRTY_SECONDS); | ||
| }); | ||
| } | ||
|
|
||
| public static void cancelAuthorizationTask(AdminClient adminClient) throws Exception { | ||
| static void cancelAuthorizationTask(AdminClient adminClient) throws Exception { | ||
| var pollerTask = waitForTask(AUTH_TASK_ACTION, adminClient); | ||
| var builder = new CancelTasksRequestBuilder(adminClient.cluster()); | ||
|
|
||
|
|
@@ -202,7 +241,11 @@ public void testCreatesEisChatCompletion_DoesNotRemoveEndpointWhenNoLongerAuthor | |
| } | ||
|
|
||
| private void assertChatCompletionEndpointExists() { | ||
| var eisEndpoints = getEisEndpoints(); | ||
| assertChatCompletionEndpointExists(modelRegistry); | ||
| } | ||
|
|
||
| static void assertChatCompletionEndpointExists(ModelRegistry modelRegistry) { | ||
| var eisEndpoints = getEisEndpoints(modelRegistry); | ||
| assertThat(eisEndpoints.size(), is(1)); | ||
|
|
||
| var rainbowSprinklesModel = eisEndpoints.get(0); | ||
|
|
@@ -212,7 +255,7 @@ private void assertChatCompletionEndpointExists() { | |
| ); | ||
| } | ||
|
|
||
| private void assertChatCompletionUnparsedModel(UnparsedModel rainbowSprinklesModel) { | ||
| static void assertChatCompletionUnparsedModel(UnparsedModel rainbowSprinklesModel) { | ||
| assertThat(rainbowSprinklesModel.taskType(), is(TaskType.CHAT_COMPLETION)); | ||
| assertThat(rainbowSprinklesModel.service(), is(ElasticInferenceService.NAME)); | ||
| assertThat(rainbowSprinklesModel.inferenceEntityId(), is(InternalPreconfiguredEndpoints.DEFAULT_CHAT_COMPLETION_ENDPOINT_ID_V1)); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1041,7 +1041,7 @@ private void assertReturnModelIsModifiable(UnparsedModel unparsedModel) { | |
| } | ||
| } | ||
|
|
||
| private Model buildElserModelConfig(String inferenceEntityId, TaskType taskType) { | ||
| static Model buildElserModelConfig(String inferenceEntityId, TaskType taskType) { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Making it available for CCMServiceIT so we can force a cluster update by creating an inference endpoint. |
||
| return switch (taskType) { | ||
| case SPARSE_EMBEDDING -> new org.elasticsearch.xpack.inference.services.elasticsearch.ElserInternalModel( | ||
| inferenceEntityId, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing these because they were originally fixed here: #137839 but that PR didn't unmute the tests.