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
ISPN-4070 Server plugin for RHQ needs extensions in #2663
Conversation
Needs rebase still which is conflicted. |
Rebased. |
|
||
@Override | ||
protected ModelNode invokeCommand(Cache<?, ?> cache, ModelNode operation) throws Exception { | ||
cache.getAdvancedCache().getComponentRegistry().start(); |
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.
This needs to be wrapped in a SecurityAction. Also, shouldn't you be invoking cache.start() and stop() instead ?
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.
Just FYI, calling operations using this approach results in the following server console output:
06:55:07,916 INFO [org.jboss.as.clustering.infinispan](XNIO-1 task-5) JBAS010282: Stopped default cache from local container
06:56:25,939 INFO [org.infinispan.eviction.impl.EvictionManagerImpl](XNIO-1 task-10) ISPN000025: wakeUpInterval is <= 0, not starting expired purge thread
06:56:25,939 INFO [org.jboss.as.clustering.infinispan](XNIO-1 task-10) JBAS010281: Started default cache from local container
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.
@tristantarrant Yeah I was wondering about the Security Action, because none of the other Cache operations were wrapped either so I left it out so we could discuss. Did you want me to add the SecurityAction for the others as well?
Also yes sorry looking again I will call directly to Cache.start/stop. I had it this way because I didn't see it on the Cache interface, forgot to check the super interfaces :)
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.
@tsykora In regards to those trace messages, they are expected currently with how the cache start/stop works. Although the INFO about wakeUpInterval could be removed by tweaking the configuration of your server, but in this case I would say not to.
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.
Yes, please add a SecurityAction for each.
Start + Stop operations work perfectly for server. Continuing with verification... |
Closing this for now as I will also cover adding in the security actions for all operations for caches and cache container. Also I will need to cover the issue of JMX operations using the proper security. |
Updated to handle additional security checks. Also testing JMX there is no issue presently as CacheImpl is directly registered in JMX and not the SecureCacheImpl wrapper. |
server2plugin.put("cluster-name", "clusterName"); | ||
server2plugin.put("cluster-size", "clusterSize"); | ||
server2plugin.put("coordinator-address", "coordinatorAddress"); | ||
server2plugin.put("local-address", "localAddress"); | ||
server2plugin.put("cache-status", "cacheStatus"); |
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.
We also need this one: server2plugin.put("cache-name", "cachename");
Just noticed... trivial change
management/monitoring * Added missing traits for cache and cache manager * Added start/stop cache operations * Added rolling upgrade operations
Remaking |
management/monitoring
https://issues.jboss.org/browse/ISPN-4070