chore(matchexpr): refactor MatchExpression as its own resource #39
Conversation
|
@tthvo review as well please, if you could |
|
I think I get a test failure after [INFO]
[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR] MatchExpressionsTest.testPostWithoutTargets:65 1 expectation failed.
JSON path data.result doesn't match.
Expected: <{expression=true, targets=[]}>
Actual: <{id=null, expression=true, targets=[]}>
[INFO]
[ERROR] Tests run: 19, Failures: 1, Errors: 0, Skipped: 1 |
|
I seem to get an error when I navigate to the Security tab after running smoketest.sh cryostat_1 | 2023-08-09 20:52:47,254 ERROR [io.qua.ver.htt.run.QuarkusErrorHandler] (executor-thread-12) HTTP Request to /api/v2.2/credentials failed, error id: b31a986a-bfa9-428c-98cb-061450da8686-7: org.hibernate.sql.exec.ExecutionException: A problem occurred in the SQL executor : Error advancing (next) ResultSet position [ResultSet is closed]
cryostat_1 | at org.hibernate.sql.results.jdbc.internal.JdbcValuesResultSetImpl.makeExecutionException(JdbcValuesResultSetImpl.java:250)
cryostat_1 | at org.hibernate.sql.results.jdbc.internal.JdbcValuesResultSetImpl.advanceNext(JdbcValuesResultSetImpl.java:207)
cryostat_1 | at org.hibernate.sql.results.jdbc.internal.JdbcValuesResultSetImpl.processNext(JdbcValuesResultSetImpl.java:84)
cryostat_1 | at org.hibernate.sql.results.jdbc.internal.AbstractJdbcValues.next(AbstractJdbcValues.java:29)
cryostat_1 | at org.hibernate.sql.results.internal.RowProcessingStateStandardImpl.next(RowProcessingStateStandardImpl.java:66)
cryostat_1 | at org.hibernate.internal.ScrollableResultsImpl.next(ScrollableResultsImpl.java:50)
cryostat_1 | at org.hibernate.query.internal.ScrollableResultsIterator.hasNext(ScrollableResultsIterator.java:33)
cryostat_1 | at java.base/java.util.Iterator.forEachRemaining(Iterator.java:132)
cryostat_1 | at java.base/java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1845)
cryostat_1 | at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
cryostat_1 | at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
cryostat_1 | at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:575)
cryostat_1 | at java.base/java.util.stream.AbstractPipeline.evaluateToArrayNode(AbstractPipeline.java:260)
cryostat_1 | at java.base/java.util.stream.ReferencePipeline.toArray(ReferencePipeline.java:616)
cryostat_1 | at java.base/java.util.stream.ReferencePipeline.toArray(ReferencePipeline.java:622)
cryostat_1 | at java.base/java.util.stream.ReferencePipeline.toList(ReferencePipeline.java:627)
cryostat_1 | at io.cryostat.credentials.Credentials.list(Credentials.java:64)
cryostat_1 | at io.cryostat.credentials.Credentials_Subclass.list$$superforward(Unknown Source)
cryostat_1 | at io.cryostat.credentials.Credentials_Subclass$$function$$2.apply(Unknown Source)
cryostat_1 | at io.quarkus.arc.impl.AroundInvokeInvocationContext.proceed(AroundInvokeInvocationContext.java:77)
cryostat_1 | at io.quarkus.arc.impl.AroundInvokeInvocationContext$NextAroundInvokeInvocationContext.proceed(AroundInvokeInvocationContext.java:103)
cryostat_1 | at io.quarkus.security.runtime.interceptor.SecurityHandler.handle(SecurityHandler.java:27)
cryostat_1 | at io.quarkus.security.runtime.interceptor.RolesAllowedInterceptor.intercept(RolesAllowedInterceptor.java:29)
cryostat_1 | at io.quarkus.security.runtime.interceptor.RolesAllowedInterceptor_Bean.intercept(Unknown Source)
cryostat_1 | at io.quarkus.arc.impl.InterceptorInvocation.invoke(InterceptorInvocation.java:42)
cryostat_1 | at io.quarkus.arc.impl.AroundInvokeInvocationContext.proceed(AroundInvokeInvocationContext.java:74)
cryostat_1 | at io.quarkus.arc.impl.AroundInvokeInvocationContext.proceed(AroundInvokeInvocationContext.java:66)
cryostat_1 | at io.quarkus.resteasy.reactive.server.runtime.StandardSecurityCheckInterceptor.intercept(StandardSecurityCheckInterceptor.java:44)
cryostat_1 | at io.quarkus.resteasy.reactive.server.runtime.StandardSecurityCheckInterceptor_RolesAllowedInterceptor_Bean.intercept(Unknown Source)
cryostat_1 | at io.quarkus.arc.impl.InterceptorInvocation.invoke(InterceptorInvocation.java:42)
cryostat_1 | at io.quarkus.arc.impl.AroundInvokeInvocationContext.perform(AroundInvokeInvocationContext.java:34)
cryostat_1 | at io.quarkus.arc.impl.InvocationContexts.performAroundInvoke(InvocationContexts.java:27)
cryostat_1 | at io.cryostat.credentials.Credentials_Subclass.list(Unknown Source)
cryostat_1 | at io.cryostat.credentials.Credentials$quarkusrestinvoker$list_ce6c13b83885b2c6f02955ea580a8e8e537fe383.invoke(Unknown Source)
cryostat_1 | at org.jboss.resteasy.reactive.server.handlers.InvocationHandler.handle(InvocationHandler.java:29)
cryostat_1 | at io.quarkus.resteasy.reactive.server.runtime.QuarkusResteasyReactiveRequestContext.invokeHandler(QuarkusResteasyReactiveRequestContext.java:141)
cryostat_1 | at org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext.run(AbstractResteasyReactiveContext.java:145)
cryostat_1 | at io.quarkus.vertx.core.runtime.VertxCoreRecorder$14.runWith(VertxCoreRecorder.java:576)
cryostat_1 | at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2513)
cryostat_1 | at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1538)
cryostat_1 | at org.jboss.threads.DelegatingRunnable.run(DelegatingRunnable.java:29)
cryostat_1 | at org.jboss.threads.ThreadLocalResettingRunnable.run(ThreadLocalResettingRunnable.java:29)
cryostat_1 | at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
cryostat_1 | at java.base/java.lang.Thread.run(Thread.java:833)
cryostat_1 | Caused by: org.hibernate.exception.GenericJDBCException: Error advancing (next) ResultSet position [ResultSet is closed] [n/a]
cryostat_1 | at org.hibernate.exception.internal.StandardSQLExceptionConverter.convert(StandardSQLExceptionConverter.java:61)
cryostat_1 | at org.hibernate.engine.jdbc.spi.SqlExceptionHelper.convert(SqlExceptionHelper.java:108)
cryostat_1 | at org.hibernate.engine.jdbc.spi.SqlExceptionHelper.convert(SqlExceptionHelper.java:94)
cryostat_1 | ... 44 more
cryostat_1 | Caused by: java.sql.SQLException: ResultSet is closed
cryostat_1 | at io.agroal.pool.wrapper.ResultSetWrapper$1.invoke(ResultSetWrapper.java:52)
cryostat_1 | at jdk.proxy3/jdk.proxy3.$Proxy94.next(Unknown Source)
cryostat_1 | at io.agroal.pool.wrapper.ResultSetWrapper.next(ResultSetWrapper.java:88)
cryostat_1 | at org.hibernate.sql.results.jdbc.internal.JdbcValuesResultSetImpl.advanceNext(JdbcValuesResultSetImpl.java:204)
cryostat_1 | ... 42 more
cryostat_1 |
cryostat_1 | 2023-08-09 20:52:47,255 INFO [io.qua.htt.access-log] (executor-thread-12) 10.89.156.16 - user 09/Aug/2023:20:52:47 +0000 "GET /api/v2.2/credentials HTTP/1.1" 500 72 |
|
I think since |
|
When I use a query like this: http --auth=user:pass POST :8181/api/beta/matchExpressions @match# match
{
"matchExpression": "true",
"targets": [{
"agent": false,
"alias": "%2Fdeployments%2Fquarkus-run.jar",
"annotations": {
"cryostat": {
"HOST": "cryostat3",
"JAVA_MAIN": "/deployments/quarkus-run.jar",
"PORT": "9091",
"REALM": "JDP"
},
"platform": {}
},
"connectUrl": "service:jmx:rmi:///jndi/rmi://cryostat3:9091/jmxrmi",
"id": 3,
"jvmId": "pUdiUY49ca7esJTZJahealZpL5eGz0UnzhdfDneLgRg=",
"labels": {}
}]
}with the correct jvmId and all that, I get but I think it should only give a response filtered from the list of targets in the json we supply? |
|
This PR/issue depends on:
|
|
I guess PR CI doesn't work because the runner doesn't have permissions to the private fork repositories. |
|
if i have a query like this: i get something like this: I think it's only caring about matching the connectUrl? Other fields seem to not matter. If that's intended, then I guess it makes sense, since we are deprecating to match on ids in the future and ignoring all these other fields anyways? |
|
I don't think I understand the example. The query has |
Sorry I meant that in the query: the other fields that I enter do not matter at all. I can have |
|
Ah, I see what you mean, thanks. I'll take a look tomorrow. My initial thought is that this probably makes sense - I think the client should only be able to ask the server to check against all targets, or against a subset of already-known targets, and in the latter case it makes sense to just provide a unique ID for them. Though, this should probably be the numeric database ID and not the connection URL. This would also need supporting changes on the client side however. This would mean the endpoint is not usable as a connection test endpoint for custom targets, though. |
…al expressions with matches
This reverts commit 8dd58e1.
|
Looks like I had left this so I guess what you're describing is (currently) expected behaviour 😁 I'll file a follow-up issue to fix this later. Either after 2.4.0 is out and this repo becomes the main focus, and -web is either retargeted for this or rolled into this, or at some other point when we have a strategy for making -web updates for 2.x vs 3.0 API differences. |
|
Reading through the implementation I'm not actually sure how it's only the connection URL that is treated as significant, however. This should end up just deserializing the request data as a |
Hm... I'm guessing when we supply the json to the endpoint, somewhere it doesn't care about if the targets are actually real, and only cares if it fits the Target specification (in this case whatever the db schema is for Target). So when we pass a target in, it executes the script correctly, just that the target may or may not be real. |
|
That is true, it just deserializes the Targets passed in the request, and doesn't actually do any checking against the database right now. I guess that would be one potential enhancement but I'm not sure it's worthwhile to do right now since the plan (before 3.0 is out, hopefully) is to revamp this to just accept that list of DB IDs anyway. |
|
Looks fine to me then, the enhancement of using ids will make this obsolete. |
Based on #34
Depends on #34
Rebased on #44
Depends on #44
WIPThis defines
MatchExpressionas a resource of its own with its own Postgres table and a couple of API endpoints (not used in the web-client or anywhere yet) for listing all the expressions and seeing more details about individual ones, including the list of matching targets. Expressions are created automatically as linked resources when either Credentials or Rules are created, and when either of these are deleted their associated Expression is also deleted.In practice this doesn't add or change much to the existing API for the other two resources that use expressions, though it does make it easier to share Expression-specific logic (ex. evaluation caching, pre-persist validation). In the future with some related web-client work I envision this being used so that server-side Expressions can be defined, listed, viewed, and deleted independently, and when creating a Rule or Credential the user can choose to either define a new Expression or choose to associate an existing Expression. This will have more performance advantages for caching and will let us build a better UX for match expressions so that they are not split between Rules and Credentials as they currently are.