Bayesian compatibility and workspace cleaning with multi-tenancy #417
Conversation
Signed-off-by: David Festal <dfestal@redhat.com>
) ... ... by: - creating a service endpoint started on wsmaster along with the Bayesian Agent, to provide the required up-to-date token (from the user that started the workspace) from the machine token of the workspace agent - retrieving this token endpoint when starting the language server on the workspace agent. Signed-off-by: David Festal <dfestal@redhat.com>
davidfestal
changed the title
| } | ||
| try { | ||
| return tokenCache.get(keycloakToken); | ||
| } catch (ExecutionException e) { | ||
| throw new OpenShiftException( | ||
| "Cound not retrieve OSO token from Keycloak token: " + keycloakToken, e.getCause()); | ||
| "Cound not retrieve OSO token from Keycloak token for user: " |
| @GET | ||
| @Path("/token") | ||
| @Produces(MediaType.APPLICATION_JSON) | ||
| public Response getBayesianToken(@HeaderParam(HttpHeaders.AUTHORIZATION) String machineToken) |
There was a problem hiding this comment.
am I correct that with getBayesianToken we do not need to have RECOMMENDER_API_TOKEN [1] as a secret in the template
| } | ||
|
|
||
| LOG.info("Recommender token : {}", recommenderToken); |
There was a problem hiding this comment.
is that token should be displayed by default ?
There was a problem hiding this comment.
no, I should have removed this after tests
| if (keycloakToken == null) { | ||
| throw new OpenShiftException("User Openshift token is needed but there is no current user"); | ||
| throw new OpenShiftException( |
There was a problem hiding this comment.
maybe add a inner method to display subject
subject.getUserName()
+ + "("
+ + subject.getUserId()
+ + ")",
as it is used several times in this class
| workspaceStarter.getUserName()); | ||
| throw new NotFoundException("Bayesian token not found"); | ||
| } | ||
| return Response.ok("\"" + keycloakToken + '"').build(); |
There was a problem hiding this comment.
you've used "\"" for the left part but '"' for the right part ?
There was a problem hiding this comment.
''"' + keycloakToken + '"' doesn't compile since the first element of the +-based concatenation should be a String. The last one can be a char, which I assume involves one less string allocation.
| public Response getBayesianToken(@HeaderParam(HttpHeaders.AUTHORIZATION) String machineToken) | ||
| throws NotFoundException { | ||
| String workspaceId = machineTokenRegistry.getWorkspaceId(machineToken); | ||
| LOG.info("workspaceId for Bayesian recommender token retrieval: {}", workspaceId); |
There was a problem hiding this comment.
it seems to me there are too many info log level on this PR but it may only myself.
Signed-off-by: David Festal <dfestal@redhat.com>
Signed-off-by: David Festal <dfestal@redhat.com>
Signed-off-by: David Festal <dfestal@redhat.com>
Signed-off-by: David Festal dfestal@redhat.com
What does this PR do?
This PR now uses the last changes made in upstream to support proper workspace cleaning (PR eclipse-che/che#7243), and make the Bayesian plugin work with multi-tenancy (issue #376) by:
Bayesian Agent, to provide the required up-to-date token (from the user
that started the workspace) from the machine token of the workspace
agent
the workspace agent.
What issues does this PR fix or reference?
#376
How have you tested this PR?
Deployed the resulting image on the AWS cluster, started a workspace on my OSIO account, got a Bayesian warning on a
pom.xmlfile.