You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Jinmei Liao <ji...@pivotal.io> on 2017/03/23 21:56:29 UTC
Review Request 57894: GEODE-2670: do not use local mbs server
connection even when pulse is in embeded mode.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57894/
-----------------------------------------------------------
Review request for geode, Anthony Baker, Jared Stewart, Kevin Duling, Ken Howe, Kirk Lund, and Patrick Rhomberg.
Repository: geode
Description
-------
* using local mbs server connection will bypass all the mbean security checks
* do not update the mbean attribute since pulse user has no cluster:write privilege at all
* Created EmbeddedPulseRule for tests
Diffs
-----
geode-assembly/src/test/java/org/apache/geode/test/dunit/rules/EmbeddedPulseRule.java PRE-CREATION
geode-assembly/src/test/java/org/apache/geode/test/dunit/rules/HttpClientRule.java d0b54b9de5759e1ba3ce78f4efaa547d391c29cf
geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseVerificationTest.java 0b3b1cef8fa54c8dcb4d2ec2e4f49e9f10d729a7
geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/PulseAppListener.java e1666ecf38331983d8c47c4b46b3b7e87faaf854
geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/controllers/PulseController.java 12b6172cbbc79651e972179532f2b79623a1992e
geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/Cluster.java e71388d134c96549ee9995c4c874615ee66fe7c1
geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/IClusterUpdater.java 3ec820769a897bfb4f10a76a86dd2ce54fba3b70
geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/JMXDataUpdater.java 9633b9a1f50df051b8fb9b4f4787a1d25a0ab019
geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/Repository.java b228e4a754100fe07c9dbec232d5e88809aefeef
geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthenticationProvider.java 4d300f04ff82f701509d44b83dd46698dbc6035e
geode-pulse/src/test/java/org/apache/geode/tools/pulse/controllers/PulseControllerJUnitTest.java ddd799f5158e1f4213b0b6b2de8e1853ba65ab74
Diff: https://reviews.apache.org/r/57894/diff/1/
Testing
-------
precheckin running
Thanks,
Jinmei Liao
Re: Review Request 57894: GEODE-2670: do not use local mbs server
connection even when pulse is in embeded mode.
Posted by Jinmei Liao <ji...@pivotal.io>.
> On March 24, 2017, 4:23 p.m., Anthony Baker wrote:
> > Did you test Pulse in non-embedded mode w/ security?
Haven't. Will try it out. Theoretically, we didn't change anythign in that aspect. It should behave the same.
> On March 24, 2017, 4:23 p.m., Anthony Baker wrote:
> > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/PulseAppListener.java
> > Line 495 (original)
> > <https://reviews.apache.org/r/57894/diff/1/?file=1673144#file1673144line495>
> >
> > Will this break pulse when not run in embedded mode?
I don't quite know why previously we need to get the username/password from a proerty file. I thought user log in with a username/password and we use the same username/password to connect to jmx. This is probably before Integrated security where you need two different sets of credentials
> On March 24, 2017, 4:23 p.m., Anthony Baker wrote:
> > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/IClusterUpdater.java
> > Lines 36 (patched)
> > <https://reviews.apache.org/r/57894/diff/1/?file=1673147#file1673147line36>
> >
> > Seems like it would be better for connect to return a JMXConnection.
True. realized that too after I committed it.
> On March 24, 2017, 4:23 p.m., Anthony Baker wrote:
> > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/IClusterUpdater.java
> > Lines 38 (patched)
> > <https://reviews.apache.org/r/57894/diff/1/?file=1673147#file1673147line38>
> >
> > I don't think a default implementation should return null.
won't need this anymore.
> On March 24, 2017, 4:23 p.m., Anthony Baker wrote:
> > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthenticationProvider.java
> > Line 67 (original)
> > <https://reviews.apache.org/r/57894/diff/1/?file=1673150#file1673150line73>
> >
> > Why is this case removed?
It's just unnessary, the exception is caught by the catch block immediatly below, so eventually, it's throwing a BadCredentialsException.
- Jinmei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57894/#review170024
-----------------------------------------------------------
On March 24, 2017, 4:03 p.m., Jinmei Liao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57894/
> -----------------------------------------------------------
>
> (Updated March 24, 2017, 4:03 p.m.)
>
>
> Review request for geode, Anthony Baker, Jared Stewart, Kevin Duling, Ken Howe, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> * using local mbs server connection will bypass all the mbean security checks
> * do not update the mbean attribute since pulse user has no cluster:write privilege at all
> * Created EmbeddedPulseRule for tests
>
>
> Diffs
> -----
>
> geode-assembly/src/test/java/org/apache/geode/test/dunit/rules/EmbeddedPulseRule.java PRE-CREATION
> geode-assembly/src/test/java/org/apache/geode/test/dunit/rules/HttpClientRule.java d0b54b9de5759e1ba3ce78f4efaa547d391c29cf
> geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseVerificationTest.java 0b3b1cef8fa54c8dcb4d2ec2e4f49e9f10d729a7
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/PulseAppListener.java e1666ecf38331983d8c47c4b46b3b7e87faaf854
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/controllers/PulseController.java 12b6172cbbc79651e972179532f2b79623a1992e
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/Cluster.java e71388d134c96549ee9995c4c874615ee66fe7c1
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/IClusterUpdater.java 3ec820769a897bfb4f10a76a86dd2ce54fba3b70
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/JMXDataUpdater.java 9633b9a1f50df051b8fb9b4f4787a1d25a0ab019
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/Repository.java b228e4a754100fe07c9dbec232d5e88809aefeef
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthenticationProvider.java 4d300f04ff82f701509d44b83dd46698dbc6035e
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/controllers/PulseControllerJUnitTest.java ddd799f5158e1f4213b0b6b2de8e1853ba65ab74
>
>
> Diff: https://reviews.apache.org/r/57894/diff/1/
>
>
> Testing
> -------
>
> precheckin successful, manul pulse testing
>
>
> Thanks,
>
> Jinmei Liao
>
>
Re: Review Request 57894: GEODE-2670: do not use local mbs server
connection even when pulse is in embeded mode.
Posted by Anthony Baker <ab...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57894/#review170024
-----------------------------------------------------------
Did you test Pulse in non-embedded mode w/ security?
geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/PulseAppListener.java
Line 495 (original)
<https://reviews.apache.org/r/57894/#comment242759>
Will this break pulse when not run in embedded mode?
geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/IClusterUpdater.java
Lines 36 (patched)
<https://reviews.apache.org/r/57894/#comment242754>
Seems like it would be better for connect to return a JMXConnection.
geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/IClusterUpdater.java
Lines 38 (patched)
<https://reviews.apache.org/r/57894/#comment242755>
I don't think a default implementation should return null.
geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthenticationProvider.java
Line 67 (original)
<https://reviews.apache.org/r/57894/#comment242757>
Why is this case removed?
- Anthony Baker
On March 24, 2017, 4:03 p.m., Jinmei Liao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57894/
> -----------------------------------------------------------
>
> (Updated March 24, 2017, 4:03 p.m.)
>
>
> Review request for geode, Anthony Baker, Jared Stewart, Kevin Duling, Ken Howe, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> * using local mbs server connection will bypass all the mbean security checks
> * do not update the mbean attribute since pulse user has no cluster:write privilege at all
> * Created EmbeddedPulseRule for tests
>
>
> Diffs
> -----
>
> geode-assembly/src/test/java/org/apache/geode/test/dunit/rules/EmbeddedPulseRule.java PRE-CREATION
> geode-assembly/src/test/java/org/apache/geode/test/dunit/rules/HttpClientRule.java d0b54b9de5759e1ba3ce78f4efaa547d391c29cf
> geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseVerificationTest.java 0b3b1cef8fa54c8dcb4d2ec2e4f49e9f10d729a7
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/PulseAppListener.java e1666ecf38331983d8c47c4b46b3b7e87faaf854
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/controllers/PulseController.java 12b6172cbbc79651e972179532f2b79623a1992e
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/Cluster.java e71388d134c96549ee9995c4c874615ee66fe7c1
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/IClusterUpdater.java 3ec820769a897bfb4f10a76a86dd2ce54fba3b70
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/JMXDataUpdater.java 9633b9a1f50df051b8fb9b4f4787a1d25a0ab019
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/Repository.java b228e4a754100fe07c9dbec232d5e88809aefeef
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthenticationProvider.java 4d300f04ff82f701509d44b83dd46698dbc6035e
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/controllers/PulseControllerJUnitTest.java ddd799f5158e1f4213b0b6b2de8e1853ba65ab74
>
>
> Diff: https://reviews.apache.org/r/57894/diff/1/
>
>
> Testing
> -------
>
> precheckin successful, manul pulse testing
>
>
> Thanks,
>
> Jinmei Liao
>
>
Re: Review Request 57894: GEODE-2670: do not use local mbs server
connection even when pulse is in embeded mode.
Posted by Jinmei Liao <ji...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57894/
-----------------------------------------------------------
(Updated March 24, 2017, 4:03 p.m.)
Review request for geode, Anthony Baker, Jared Stewart, Kevin Duling, Ken Howe, Kirk Lund, and Patrick Rhomberg.
Repository: geode
Description
-------
* using local mbs server connection will bypass all the mbean security checks
* do not update the mbean attribute since pulse user has no cluster:write privilege at all
* Created EmbeddedPulseRule for tests
Diffs
-----
geode-assembly/src/test/java/org/apache/geode/test/dunit/rules/EmbeddedPulseRule.java PRE-CREATION
geode-assembly/src/test/java/org/apache/geode/test/dunit/rules/HttpClientRule.java d0b54b9de5759e1ba3ce78f4efaa547d391c29cf
geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseVerificationTest.java 0b3b1cef8fa54c8dcb4d2ec2e4f49e9f10d729a7
geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/PulseAppListener.java e1666ecf38331983d8c47c4b46b3b7e87faaf854
geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/controllers/PulseController.java 12b6172cbbc79651e972179532f2b79623a1992e
geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/Cluster.java e71388d134c96549ee9995c4c874615ee66fe7c1
geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/IClusterUpdater.java 3ec820769a897bfb4f10a76a86dd2ce54fba3b70
geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/JMXDataUpdater.java 9633b9a1f50df051b8fb9b4f4787a1d25a0ab019
geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/Repository.java b228e4a754100fe07c9dbec232d5e88809aefeef
geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthenticationProvider.java 4d300f04ff82f701509d44b83dd46698dbc6035e
geode-pulse/src/test/java/org/apache/geode/tools/pulse/controllers/PulseControllerJUnitTest.java ddd799f5158e1f4213b0b6b2de8e1853ba65ab74
Diff: https://reviews.apache.org/r/57894/diff/1/
Testing (updated)
-------
precheckin successful, manul pulse testing
Thanks,
Jinmei Liao