Re: Review Request 53784: GEODE-1247: unable to stop server using http connection

[Jinmei Liao]

> On Nov. 15, 2016, 4:43 p.m., Kevin Duling wrote:
> > Why would we move away from using Jackson in favor of our own JSON mapping 
> > code?  Or is this a special case where we do some magic serialization we 
> > can't do via Jackson?
> 
> Jinmei Liao wrote:
> I tried using Jackson's converter, but it's giving errors about no 
> default constructor on one of the object it's trying to convert to. 
> Apparently, Jackson's converter requires the object either have some 
> annotation or a no-param constructor in order for it to work, but our own 
> doesn't.
> 
> Kirk Lund wrote:
> Is this temporary? We want to eventually delete the non-Jackson JSON 
> parser and change our code to use Jackson everywhere.

yeah, I believe we have that story somewhere. This is just to fix this error. 
Don't want to go down that rabbit hole.


- Jinmei


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53784/#review155927
-------


On Nov. 15, 2016, 3:06 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53784/
> ---
> 
> (Updated Nov. 15, 2016, 3:06 p.m.)
> 
> 
> Review request for geode, Jens Deppe, John Blum, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1247: unable to stop server using http connection
> 
> I believe after we added the "com.fasterxml.jackson.databind.ObjectMapper" 
> into core, the a mapper is added to the RestTemplate to precedes our own 
> message converter (SerializableObjectHttpMessageConverter), so on the client 
> side, we are using Jackson's converter to convert the message, but on the 
> server side, we are using our own converter, this caused the message corrupt 
> error. Fix is to remove the Jackson's converter from the client side.
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/internal/util/IOUtils.java 
> 4c1cc5f79a3bed922347835441f6624931bdcc6b 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/web/shell/AbstractHttpOperationInvoker.java
>  25eee8295ac553da416eb928b160fe7897b17763 
>   
> geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/QueryNamesOverHttpDUnitTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53784/diff/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 53784: GEODE-1247: unable to stop server using http connection

[Jinmei Liao]

> On Nov. 15, 2016, 4:43 p.m., Kevin Duling wrote:
> > Why would we move away from using Jackson in favor of our own JSON mapping 
> > code?  Or is this a special case where we do some magic serialization we 
> > can't do via Jackson?

I tried using Jackson's converter, but it's giving errors about no default 
constructor on one of the object it's trying to convert to. Apparently, 
Jackson's converter requires the object either have some annotation or a 
no-param constructor in order for it to work, but our own doesn't.


- Jinmei


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53784/#review155927
---


On Nov. 15, 2016, 3:06 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53784/
> ---
> 
> (Updated Nov. 15, 2016, 3:06 p.m.)
> 
> 
> Review request for geode, Jens Deppe, John Blum, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1247: unable to stop server using http connection
> 
> I believe after we added the "com.fasterxml.jackson.databind.ObjectMapper" 
> into core, the a mapper is added to the RestTemplate to precedes our own 
> message converter (SerializableObjectHttpMessageConverter), so on the client 
> side, we are using Jackson's converter to convert the message, but on the 
> server side, we are using our own converter, this caused the message corrupt 
> error. Fix is to remove the Jackson's converter from the client side.
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/internal/util/IOUtils.java 
> 4c1cc5f79a3bed922347835441f6624931bdcc6b 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/web/shell/AbstractHttpOperationInvoker.java
>  25eee8295ac553da416eb928b160fe7897b17763 
>   
> geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/QueryNamesOverHttpDUnitTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53784/diff/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: gfsh start server with the --password option

[Jinmei Liao]

I thought we had code that deals with redacting password in gfsh history,
not sure why it's not in effect anymore.

On Tue, Nov 15, 2016 at 2:27 PM, Swapnil Bawaskar 
wrote:

> When you want to connect to a secure system you can choose not to use the
> --password option at which point you will be prompted to enter a
> username/password.
> e.g:
> gfsh>connect --locator=localhost[10334]
> Connecting to Locator at [host=localhost, port=10334] ..
> Connecting to Manager at [host=192.168.1.181, port=1099] ..
> username: super-user
> password: 
>
>
> On Tue, Nov 15, 2016 at 11:55 AM, Kirk Lund  wrote:
>
> > There should be redaction in gfsh history. Maybe repeating the command
> is a
> > case that wasn't fully covered? This is a bug we'll need to file and fix.
> >
> > Clear text in process string is probably not a bug. Users should
> implement
> > a callback to provide the password instead of providing it as a system
> > property unless they're ok with it showing in the process string. This
> may
> > need more documentation?
> >
> > The logs should not contain the clear text password and this would be a
> bug
> > if it does.
> >
> > -Kirk
> >
> >
> > On Tue, Nov 15, 2016 at 11:08 AM, Karen Miller 
> wrote:
> >
> > > When specifying user name and password to use as authentication
> > credentials
> > > with the gfsh start server command, the password is specified in the
> > clear.
> > > I've added a note in the documentation to point this out, but
> specifying
> > a
> > > password
> > > in this way leads to further ways the clear text password can be seen.
> > >
> > > - gfsh history will repeat back the command with the password shown
> > > - any user on the box can see the clear text password with 'ps'
> > > - (haven't checked if this happens) logs may have the clear text
> password
> > >
> > > Is this an issue?  The history is for a particular user, so not so bad.
> > > Logs can use file system permissions to reduce access.  But anyone with
> > > access to the box can list the processes.
> > >
> > > Karen
> > >
> >
>



-- 
Cheers

Jinmei

Review Request 53784: GEODE-1247: unable to stop server using http connection

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53784/
---

Review request for geode, Jens Deppe, John Blum, Kevin Duling, and Kirk Lund.


Repository: geode


Description
---

GEODE-1247: unable to stop server using http connection

I believe after we added the "com.fasterxml.jackson.databind.ObjectMapper" into 
core, the a mapper is added to the RestTemplate to precedes our own message 
converter (SerializableObjectHttpMessageConverter), so on the client side, we 
are using Jackson's converter to convert the message, but on the server side, 
we are using our own converter, this caused the message corrupt error. Fix is 
to remove the Jackson's converter from the client side.


Diffs
-

  geode-core/src/main/java/org/apache/geode/internal/util/IOUtils.java 
4c1cc5f79a3bed922347835441f6624931bdcc6b 
  
geode-core/src/main/java/org/apache/geode/management/internal/web/shell/AbstractHttpOperationInvoker.java
 25eee8295ac553da416eb928b160fe7897b17763 
  
geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/QueryNamesOverHttpDUnitTest.java
 PRE-CREATION 

Diff: https://reviews.apache.org/r/53784/diff/


Testing
---

precheckin running


Thanks,

Jinmei Liao

Re: Review Request 53750: GEODE-2110 Add gfsh start server user and password options

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53750/#review155879
---


Ship it!




Ship It!

- Jinmei Liao


On Nov. 15, 2016, 1:55 a.m., Karen Miller wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53750/
> ---
> 
> (Updated Nov. 15, 2016, 1:55 a.m.)
> 
> 
> Review request for geode, Dave Barnes, Jinmei Liao, and Joey McAllister.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Modifies gfsh start server command reference page to add
> the two missing options: --user and --password
> 
> 
> Diffs
> -
> 
>   geode-docs/tools_modules/gfsh/command-pages/start.html.md.erb 
> 06dfce4d6a96f64295a069a50d79d55b87a1da67 
> 
> Diff: https://reviews.apache.org/r/53750/diff/
> 
> 
> Testing
> ---
> 
> 1. bundle exec bookbinder bind local   finds no broken links
> 
> 2. gradle rat check passes
> 
> 
> Thanks,
> 
> Karen Miller
> 
>

Re: Review Request 53685: GEODE-1955: properly disconnect gfsh session so that it won't leave heartbeat thread around to pollute other tests

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53685/
---

(Updated Nov. 14, 2016, 8:56 p.m.)


Review request for geode, Kevin Duling, Kirk Lund, and Udo Kohlmeyer.


Repository: geode


Description
---

* add the "disconnect" command when we close the GfshShellConnectionRule so 
that no heartbeat thread is left to pollute other tests.
* Fix the test so that it truely test the jmx ssl connection, and fix the 
configuration mishaps.
* The above fix revealed another bug (GEODE-2099: race condition), ignored the 
other two tests for now.


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/internal/net/SSLConfigurationFactory.java
 76fb04169f06f82022858cbd0a03eadac8a42ef6 
  
geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerAdvisee.java
 0467c486a0e343bf745fe743172ed4fce750c5b4 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java
 456f76bb5592b000c75fe733553219c09d1b5381 
  
geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java
 b6afcca7cf8a27c2ad90d6ea570755a63ac0e89b 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java
 44da08be416240f49cbe9dfb8653f41eaea8777e 

Diff: https://reviews.apache.org/r/53685/diff/


Testing
---


Thanks,

Jinmei Liao

Review Request 53685: GEODE-1955: properly disconnect gfsh session so that it won't leave heartbeat thread around to pollute other tests

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53685/
---

Review request for geode, Kevin Duling, Kirk Lund, and Udo Kohlmeyer.


Repository: geode


Description
---

* add the "disconnect" command when we close the GfshShellConnectionRule so 
that no heartbeat thread is left to pollute other tests.
* Fix the test so that it truely test the jmx ssl connection, and fix the 
configuration mishaps.
* The above fix revealed another bug (GEODE-2099: race condition), ignored the 
other two tests for now.


Diffs
-

  
geode-core/src/main/java/org/apache/geode/internal/net/SSLConfigurationFactory.java
 76fb04169f06f82022858cbd0a03eadac8a42ef6 
  
geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerAdvisee.java
 0467c486a0e343bf745fe743172ed4fce750c5b4 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java
 456f76bb5592b000c75fe733553219c09d1b5381 
  
geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java
 b6afcca7cf8a27c2ad90d6ea570755a63ac0e89b 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java
 44da08be416240f49cbe9dfb8653f41eaea8777e 

Diff: https://reviews.apache.org/r/53685/diff/


Testing
---


Thanks,

Jinmei Liao

Re: Review Request 53661: GEODE-2094 Update admin/dev REST API documentation

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53661/#review155747
---


Ship it!




Ship It!

- Jinmei Liao


On Nov. 11, 2016, 5:37 p.m., Karen Miller wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53661/
> ---
> 
> (Updated Nov. 11, 2016, 5:37 p.m.)
> 
> 
> Review request for geode, Dave Barnes, Jinmei Liao, Joey McAllister, and 
> Kevin Duling.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> - Add 3 missing gfsh start server options:
>   --http-service-port
>   --http-service-bind-address
>   --start-rest-api
> 
> - Update examples to use these options, instead of using
> the older --J=-gemfire. specification.
> 
> 
> Diffs
> -
> 
>   geode-docs/rest_apps/setup_config.html.md.erb 
> 604dd059290f8da41e2e05e0946e6a7eaa01e6ca 
> 
> Diff: https://reviews.apache.org/r/53661/diff/
> 
> 
> Testing
> ---
> 
> gradle rat check passes
> 
> 
> Thanks,
> 
> Karen Miller
> 
>

Re: Where should geode example code live

[Jinmei Liao]

+1 for both renaming it now and moving it to a separate module going
forward.

On Thu, Nov 10, 2016 at 10:58 AM, Kirk Lund  wrote:

> We have some example code that's not in geode-examples.
> geode-core-1.0.0-incubating.jar currently contains this package with
> examples:
>
> org.geode.gemfire.security.templates
>
> Given that template means something a bit different from example, I'd like
> to rename this package to examples. Is this acceptable?
>
> Going forward, this community needs to spend some considerable effort in
> improving modularity of geode. The geode-core module should contain only
> the bare essentials that are always necessary when using geode. Optional
> features and the like should move to their own modules. Along the same line
> of thought, examples should move to geode-examples or some similar
> sub-module (or even a separate repo). That sub-module should generate a jar
> that can be added to the classpath for purposes of demoing or trying out
> geode (kicking the tires). I'd like the community to come up with
> additional ideas on how to facilitate demoing and also make it easier in
> general to try out geode for the first time.
>
> -Kirk
>



-- 
Cheers

Jinmei

Review Request 53587: GEODE-2014: explicitly declare spring-web as a direct dependency rather than a transitive one.

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53587/
---

Review request for geode, Anthony Baker, Kevin Duling, and Kirk Lund.


Repository: geode


Description
---

GEODE-2014: explicitly declare spring-web as a direct dependency rather than a 
transitive one.


Diffs
-

  geode-web-api/build.gradle 9b1551633fc08296aab75dde4c251d086f583f82 

Diff: https://reviews.apache.org/r/53587/diff/


Testing
---

dependency check


Thanks,

Jinmei Liao

Re: Review Request 53579: GEODE-1570: add a test to verify rest security with SSL.

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53579/
---

(Updated Nov. 8, 2016, 8:53 p.m.)


Review request for geode, Kevin Duling and Kirk Lund.


Repository: geode


Description
---

GEODE-1570: add a test to verify rest security with SSL.


Diffs (updated)
-

  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/GeodeRestClient.java
 071b95ca8096f1e7872c9780fbf9c23bad92dc25 
  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityWithSSLTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/security/SecurityClusterConfigDUnitTest.java
 cce4fdbb2e3a35fd0727b74baf07132d77a324dd 

Diff: https://reviews.apache.org/r/53579/diff/


Testing (updated)
---

precheckin successful


Thanks,

Jinmei Liao

Re: Review Request 53577: Remove illegal javadoc tag that generates warning

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53577/#review155328
---


Ship it!




Ship It!

- Jinmei Liao


On Nov. 8, 2016, 5:54 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53577/
> ---
> 
> (Updated Nov. 8, 2016, 5:54 p.m.)
> 
> 
> Review request for geode, Jinmei Liao and Kevin Duling.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> @return and @param tags must have string value or it's illegal and generates 
> warning.
> 
> This change deletes an @return javadoc that has no value.
> 
> Here's the warning that the build reports:
> 
> :geode-core:javadoc
> /export/latvia1/users/klund/dev/gemfire/open/geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java:447:
>  warning - @return tag has no arguments.
> 1 warning
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
>  7a898d13a387a8551ac6f6e5433891f6db3653d2 
> 
> Diff: https://reviews.apache.org/r/53577/diff/
> 
> 
> Testing
> ---
> 
> build javadoc
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Re: Review Request 53579: GEODE-1570: add a test to verify rest security with SSL.

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53579/
---

(Updated Nov. 8, 2016, 6:13 p.m.)


Review request for geode, Kevin Duling and Kirk Lund.


Repository: geode


Description
---

GEODE-1570: add a test to verify rest security with SSL.


Diffs
-

  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/GeodeRestClient.java
 071b95ca8096f1e7872c9780fbf9c23bad92dc25 
  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityWithSSLTest.java
 PRE-CREATION 
  geode-assembly/src/test/resources/ssl/trusted.keystore PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarterRule.java
 72627a08ab5bbd6756c81b2bc332e97ac8692976 

Diff: https://reviews.apache.org/r/53579/diff/


Testing (updated)
---

precheckin


Thanks,

Jinmei Liao

Review Request 53579: GEODE-1570: add a test to verify rest security with SSL.

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53579/
---

Review request for geode.


Repository: geode


Description
---

GEODE-1570: add a test to verify rest security with SSL.


Diffs
-

  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/GeodeRestClient.java
 071b95ca8096f1e7872c9780fbf9c23bad92dc25 
  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityWithSSLTest.java
 PRE-CREATION 
  geode-assembly/src/test/resources/ssl/trusted.keystore PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarterRule.java
 72627a08ab5bbd6756c81b2bc332e97ac8692976 

Diff: https://reviews.apache.org/r/53579/diff/


Testing
---


Thanks,

Jinmei Liao

Review Request 53543: GEODE-2079: mark the test as flaky

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53543/
---

Review request for geode, Kevin Duling and Kirk Lund.


Repository: geode


Description
---

GEODE-2079: mark the test as flaky


Diffs
-

  
geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java
 1c8079aafa32b2848dfae573f2cd4e8e9f28cc5b 

Diff: https://reviews.apache.org/r/53543/diff/


Testing
---


Thanks,

Jinmei Liao

Re: Build failed in Jenkins: Geode-nightly #646

[Jinmei Liao]

I will take care of this.

On Mon, Nov 7, 2016 at 7:42 AM, Apache Jenkins Server <
jenk...@builds.apache.org> wrote:

> See 
>
> --
> [...truncated 526 lines...]
> :geode-cq:check
> :geode-cq:build
> :geode-cq:distributedTest
> :geode-cq:flakyTest
> :geode-cq:integrationTest
> :geode-json:assemble
> :geode-json:compileTestJava UP-TO-DATE
> :geode-json:processTestResources UP-TO-DATE
> :geode-json:testClasses UP-TO-DATE
> :geode-json:checkMissedTests UP-TO-DATE
> :geode-json:spotlessJavaCheck
> :geode-json:spotlessCheck
> :geode-json:test UP-TO-DATE
> :geode-json:check
> :geode-json:build
> :geode-json:distributedTest UP-TO-DATE
> :geode-json:flakyTest UP-TO-DATE
> :geode-json:integrationTest UP-TO-DATE
> :geode-junit:javadoc
> :geode-junit:javadocJar
> :geode-junit:sourcesJar
> :geode-junit:signArchives SKIPPED
> :geode-junit:assemble
> :geode-junit:compileTestJava
> :geode-junit:processTestResources UP-TO-DATE
> :geode-junit:testClasses
> :geode-junit:checkMissedTests
> :geode-junit:spotlessJavaCheck
> :geode-junit:spotlessCheck
> :geode-junit:test
> :geode-junit:check
> :geode-junit:build
> :geode-junit:distributedTest
> :geode-junit:flakyTest
> :geode-junit:integrationTest
> :geode-lucene:assemble
> :geode-lucene:compileTestJavaNote: Some input files use or override a
> deprecated API.
> Note: Recompile with -Xlint:deprecation for details.
> Note: Some input files use unchecked or unsafe operations.
> Note: Recompile with -Xlint:unchecked for details.
>
> :geode-lucene:processTestResources
> :geode-lucene:testClasses
> :geode-lucene:checkMissedTests
> :geode-lucene:spotlessJavaCheck
> :geode-lucene:spotlessCheck
> :geode-lucene:test
> :geode-lucene:check
> :geode-lucene:build
> :geode-lucene:distributedTest
> :geode-lucene:flakyTest
> :geode-lucene:integrationTest
> :geode-old-client-support:assemble
> :geode-old-client-support:compileTestJava
> :geode-old-client-support:processTestResources UP-TO-DATE
> :geode-old-client-support:testClasses
> :geode-old-client-support:checkMissedTests
> :geode-old-client-support:spotlessJavaCheck
> :geode-old-client-support:spotlessCheck
> :geode-old-client-support:test
> :geode-old-client-support:check
> :geode-old-client-support:build
> :geode-old-client-support:distributedTest
> :geode-old-client-support:flakyTest
> :geode-old-client-support:integrationTest
> :geode-pulse:assemble
> :geode-pulse:compileTestJavaNote:  job/Geode-nightly/ws/geode-pulse/src/test/java/org/
> apache/geode/tools/pulse/tests/ui/PulseAbstractTest.java> uses or
> overrides a deprecated API.
> Note: Recompile with -Xlint:deprecation for details.
> Note: Some input files use unchecked or unsafe operations.
> Note: Recompile with -Xlint:unchecked for details.
>
> :geode-pulse:processTestResources
> :geode-pulse:testClasses
> :geode-pulse:checkMissedTests
> :geode-pulse:spotlessJavaCheck
> :geode-pulse:spotlessCheck
> :geode-pulse:test
> :geode-pulse:check
> :geode-pulse:build
> :geode-pulse:distributedTest
> :geode-pulse:flakyTest
> :geode-pulse:integrationTest
> :geode-rebalancer:assemble
> :geode-rebalancer:compileTestJava
> :geode-rebalancer:processTestResources UP-TO-DATE
> :geode-rebalancer:testClasses
> :geode-rebalancer:checkMissedTests
> :geode-rebalancer:spotlessJavaCheck
> :geode-rebalancer:spotlessCheck
> :geode-rebalancer:test
> :geode-rebalancer:check
> :geode-rebalancer:build
> :geode-rebalancer:distributedTest
> :geode-rebalancer:flakyTest
> :geode-rebalancer:integrationTest
> :geode-wan:assemble
> :geode-wan:compileTestJavaNote: Some input files use or override a
> deprecated API.
> Note: Recompile with -Xlint:deprecation for details.
> Note: Some input files use unchecked or unsafe operations.
> Note: Recompile with -Xlint:unchecked for details.
>
> :geode-wan:processTestResources
> :geode-wan:testClasses
> :geode-wan:checkMissedTests
> :geode-wan:spotlessJavaCheck
> :geode-wan:spotlessCheck
> :geode-wan:test
> :geode-wan:check
> :geode-wan:build
> :geode-wan:distributedTest
> :geode-wan:flakyTest
> :geode-wan:integrationTest
> :geode-web:assemble
> :geode-web:compileTestJavaNote: Some input files use or override a
> deprecated API.
> Note: Recompile with -Xlint:deprecation for details.
> Note: Some input files use unchecked or unsafe operations.
> Note: Recompile with -Xlint:unchecked for details.
>
> :geode-web:processTestResources UP-TO-DATE
> :geode-web:testClasses
> :geode-web:checkMissedTests
> :geode-web:spotlessJavaCheck
> :geode-web:spotlessCheck
> :geode-web:test
> :geode-web:check
> :geode-web:build
> :geode-web:distributedTest
> :geode-web:flakyTest
> :geode-web:integrationTest
> :geode-web-api:assemble
> :geode-web-api:compileTestJava UP-TO-DATE
> :geode-web-api:processTestResources UP-TO-DATE
> :geode-web-api:testClasses UP-TO-DATE
> :geode-web-api:checkMissedTests UP-TO-DATE
> :geode-web-api:spotlessJavaCheck
> :geode-web-api:spotlessCheck
> 

Re: Gfsh Parsing

[Jinmei Liao]

This is a good idea. I'll reach out to find it out. Thanks!

On Fri, Nov 4, 2016 at 9:48 AM, Michael Stolz <mst...@pivotal.io> wrote:

> Can we suggest improvements to the Spring help capabilities? The Spring
> community tends to be very responsive to good suggestions.
>
> --
> Mike Stolz
> Principal Engineer - Gemfire Product Manager
> Mobile: 631-835-4771
> On Nov 4, 2016 8:27 AM, "Jinmei Liao" <jil...@pivotal.io> wrote:
>
> > We have several jira issues related to gfsh parsing (GEODE-1598,
> > GEODE-1912). After spending some time understanding how the parsing
> works,
> > I found out we have three components intertwined together all trying to
> do
> > parsing: Gfsh, JoptSimple and Spring Shell. I started an experiment by
> > getting rid of Gfsh and JoptSimple parsing and only using Spring Shell.
> The
> > outcome is I am able to maintain the current parsing and tabbing
> completion
> > capabilities (and fix a few bugs) by removing 40+ files. The only
> > difference I see so far lies in the help and hint messages. It seems the
> > main reason we are using these home backed Gfsh parsing is to provide
> more
> > readable help messages. Below are the differences:
> >
> > Using Spring Shell's provided help:
> >
> > Using Gfsh Parsing with JoptSimple:
> >
> >
> > ​
> > ​I do like the outcome of the latter, but added complexity of the code is
> > too expensive to bear. So I am asking the community how important it is
> to
> > maintain the current style of help? Can we do with the cheaper way by
> just
> > using whatever provided by the libraries?
> >
> > --
> > Cheers
> >
> > Jinmei
> >
>



-- 
Cheers

Jinmei

Re: Gfsh Parsing

[Jinmei Liao]

Hi, it would be nice to know what kind of GFSH Parser api you are using?
Are you specifically using GfshParser or just the execution result?

On Fri, Nov 4, 2016 at 9:51 AM, Real Wes <thereal...@outlook.com> wrote:

> I call the GFSH Parser from code and rely on the formatting of the return
> message to determine the response. So I’d like to see that code as
> encapsulated in one place.
>
> > On Nov 4, 2016, at 11:38 AM, Jinmei Liao <jil...@pivotal.io> wrote:
> >
> >
> > We have several jira issues related to gfsh parsing (GEODE-1598,
> > GEODE-1912). After spending some time understanding how the parsing
> works,
> > I found out we have three components intertwined together all trying to
> do
> > parsing: Gfsh, JoptSimple and Spring Shell. I started an experiment by
> > getting rid of Gfsh and JoptSimple parsing and only using Spring Shell.
> The
> > outcome is I am able to maintain the current parsing and tabbing
> completion
> > capabilities (and fix a few bugs) by removing 40+ files. The only
> > difference I see so far lies in the help and hint messages. It seems the
> > main reason we are using these home backed Gfsh parsing is to provide
> more
> > readable help messages. Below are the differences:
> >
> > Using Spring Shell's provided help:
> >
> > Using Gfsh Parsing with JoptSimple:
> >
> >
> > I do like the outcome of the latter, but added complexity of the code is
> > too expensive to bear. So I am asking the community how important it is
> to
> > maintain the current style of help? Can we do with the cheaper way by
> just
> > using whatever provided by the libraries?
> >
> > Cheers
> >
> > Jinmei
> >
>
>


-- 
Cheers

Jinmei

Gfsh Parsing

[Jinmei Liao]

We have several jira issues related to gfsh parsing (GEODE-1598,
GEODE-1912). After spending some time understanding how the parsing works,
I found out we have three components intertwined together all trying to do
parsing: Gfsh, JoptSimple and Spring Shell. I started an experiment by
getting rid of Gfsh and JoptSimple parsing and only using Spring Shell. The
outcome is I am able to maintain the current parsing and tabbing completion
capabilities (and fix a few bugs) by removing 40+ files. The only
difference I see so far lies in the help and hint messages. It seems the
main reason we are using these home backed Gfsh parsing is to provide  more
readable help messages. Below are the differences:

Using Spring Shell's provided help:

Using Gfsh Parsing with JoptSimple:


​
​I do like the outcome of the latter, but added complexity of the code is
too expensive to bear. So I am asking the community how important it is to
maintain the current style of help? Can we do with the cheaper way by just
using whatever provided by the libraries?

-- 
Cheers

Jinmei

Re: Review Request 53274: GEODE-1955: fix flaky tests by properly closing the gfsh instance in ConnectToLocatorSSLDUnitTest

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53274/
---

(Updated Nov. 3, 2016, 7:38 p.m.)


Review request for geode, Kevin Duling and Kirk Lund.


Repository: geode


Description
---

GEODE-1955: fix flaky tests by properly closing the gfsh instance in 
ConnectToLocatorSSLDUnitTest


Diffs
-

  
geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java
 8426c6f52129472c9c0def172aeefd10f1491935 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java
 d3390ba6ce2990d05516b701f82d5f176e6f4d49 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java
 b2dc6fe334499456184e1d0cbc0297063d4c99ce 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java
 25780e67dd3f7920e53162ef9400d2845e4c958b 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
 4e10b0acae6c6aac88b6039560e4cf65b0a012cb 
  
geode-web/src/test/java/org/apache/geode/management/internal/security/GfshCommandsOverHttpSecurityTest.java
 d4775659c0ff2704fd55a3840fc3751e66886d26 

Diff: https://reviews.apache.org/r/53274/diff/


Testing (updated)
---

precheckin successful


Thanks,

Jinmei Liao

Re: Review Request 53434: GEODE-2060 Update docs for security-related poperties

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53434/#review154750
---


Ship it!




Ship It!

- Jinmei Liao


On Nov. 3, 2016, 4:24 p.m., Karen Miller wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53434/
> ---
> 
> (Updated Nov. 3, 2016, 4:24 p.m.)
> 
> 
> Review request for geode, Dave Barnes, Jinmei Liao, and Joey McAllister.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Add security-manager and security-post-processor.
> Deprecate others:
>   security-client-accessor
>   security-client-accessor-pp
>   security-client-authenticator
>   security-peer-auth-init
>   security-peer-authenticator
> 
> 
> Diffs
> -
> 
>   geode-docs/reference/topics/gemfire_properties.html.md.erb 
> ae0f198b1f765469801958e0f7854b54b803b003 
> 
> Diff: https://reviews.apache.org/r/53434/diff/
> 
> 
> Testing
> ---
> 
> gradle rat check passes
> 
> 
> Thanks,
> 
> Karen Miller
> 
>

Re: Review Request 53402: GEODE-2013: throw IllegalStateException if resource type is missing

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53402/#review154629
---


Ship it!




Ship It!

- Jinmei Liao


On Nov. 2, 2016, 8:07 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53402/
> ---
> 
> (Updated Nov. 2, 2016, 8:07 p.m.)
> 
> 
> Review request for geode, Anthony Baker, Darrel Schneider, Jinmei Liao, Jared 
> Stewart, and Kevin Duling.
> 
> 
> Bugs: GEODE-2013
> https://issues.apache.org/jira/browse/GEODE-2013
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2013: throw IllegalStateException if resource type is missing
> 
> The StatArchiveReader was throwing NullPointerException if it found a 
> Resource without a ResourceType. Now it throws IllegalStateException with a 
> message explaining that no ResourceType for the specified resourceTypeId 
> could be found.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/statistics/StatArchiveReader.java
>  ca1a6c49269a56600191ac7b0551670ffb0f2c82 
>   
> geode-core/src/test/java/org/apache/geode/internal/statistics/StatArchiveWithMissingResourceTypeRegressionTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/resources/org/apache/geode/internal/statistics/StatArchiveWithMissingResourceTypeRegressionTest.gfs
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53402/diff/
> 
> 
> Testing
> ---
> 
> StatArchiveWithMissingResourceTypeRegressionTest
> precheckin
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Re: Review Request 53274: GEODE-1955: fix flaky tests by properly closing the gfsh instance in ConnectToLocatorSSLDUnitTest

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53274/
---

(Updated Nov. 1, 2016, 5:19 p.m.)


Review request for geode, Kevin Duling and Kirk Lund.


Repository: geode


Description
---

GEODE-1955: fix flaky tests by properly closing the gfsh instance in 
ConnectToLocatorSSLDUnitTest


Diffs (updated)
-

  
geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java
 8426c6f52129472c9c0def172aeefd10f1491935 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java
 d3390ba6ce2990d05516b701f82d5f176e6f4d49 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java
 b2dc6fe334499456184e1d0cbc0297063d4c99ce 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java
 25780e67dd3f7920e53162ef9400d2845e4c958b 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
 4e10b0acae6c6aac88b6039560e4cf65b0a012cb 
  
geode-web/src/test/java/org/apache/geode/management/internal/security/GfshCommandsOverHttpSecurityTest.java
 d4775659c0ff2704fd55a3840fc3751e66886d26 

Diff: https://reviews.apache.org/r/53274/diff/


Testing
---


Thanks,

Jinmei Liao

Re: Review Request 53274: GEODE-1955: fix flaky tests by properly closing the gfsh instance in ConnectToLocatorSSLDUnitTest

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53274/
---

(Updated Nov. 1, 2016, 5:18 p.m.)


Review request for geode, Kevin Duling and Kirk Lund.


Repository: geode


Description
---

GEODE-1955: fix flaky tests by properly closing the gfsh instance in 
ConnectToLocatorSSLDUnitTest


Diffs (updated)
-

  
geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java
 8426c6f52129472c9c0def172aeefd10f1491935 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java
 25780e67dd3f7920e53162ef9400d2845e4c958b 

Diff: https://reviews.apache.org/r/53274/diff/


Testing
---


Thanks,

Jinmei Liao

Review Request 53274: GEODE-1955: fix flaky tests by properly closing the gfsh instance in ConnectToLocatorSSLDUnitTest

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53274/
---

Review request for geode, Kevin Duling and Kirk Lund.


Repository: geode


Description
---

GEODE-1955: fix flaky tests by properly closing the gfsh instance in 
ConnectToLocatorSSLDUnitTest


Diffs
-

  
geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java
 8426c6f52129472c9c0def172aeefd10f1491935 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java
 d3390ba6ce2990d05516b701f82d5f176e6f4d49 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java
 b2dc6fe334499456184e1d0cbc0297063d4c99ce 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java
 25780e67dd3f7920e53162ef9400d2845e4c958b 

Diff: https://reviews.apache.org/r/53274/diff/


Testing
---


Thanks,

Jinmei Liao

Review Request 53241: GEODE-1912: make ServerStarter and LocatorStarter as regular rules so that it's easier to use them in a RuleChain

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53241/
---

Review request for geode, Kevin Duling and Kirk Lund.


Repository: geode


Description
---

GEODE-1912: make ServerStarter and LocatorStarter as regular rules so that it's 
easier to use them in a RuleChain

* refactor gfshConectionRule as well


Diffs
-

  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java
 2dffcb713d761ace4b39aa9cf58fdea1d510af0b 
  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestServersJUnitTest.java
 552a184bb9fd47dc70f1d0ab6022af18aaa7233e 
  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/SwaggerVerificationTest.java
 55cc26e28e44bc3be6beeb11e441b55046fcc3a8 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java
 bc25567e12e35e98d74edbd68acda7379503a895 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanShiroJUnitTest.java
 992c27d5e2216879d66d9b8f1425ec2b5de0eeb1 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerStartupRule.java
 c9ae99700061865874fbcc691a0281f2cbf5075b 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java
 60f58da8e1d047b9dde772d60eba4ada4428e9af 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java
 ce9c21c202d35202fb4b8bdf84fa90b100fa2c47 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/GfshShellConnectionRule.java
 e7724f75fd23f3530b6b2e3214b6b1bfb2d1cca9 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/JavaRmiServerNameTest.java
 f754f2e04fe638d6c9b4cc9c31f513f4b47613ea 
  
geode-core/src/test/java/org/apache/geode/security/AbstractSecureServerDUnitTest.java
 b9efe072d4074a5145efa74b5e69338e16f1d35c 
  
geode-core/src/test/java/org/apache/geode/security/ClusterConfigWithoutSecurityDUnitTest.java
 cbe4a3bb4204fcf0e5482efb4e682bb9633eee04 
  
geode-core/src/test/java/org/apache/geode/security/PeerAuthenticatorDUnitTest.java
 84a4dd457274e3cd6cf1e632842ffa73846abad9 
  
geode-core/src/test/java/org/apache/geode/security/PeerSecurityWithEmbeddedLocatorDUnitTest.java
 577a7a1d41476be87f35d5abbba68617d5e8db51 
  
geode-core/src/test/java/org/apache/geode/security/SecurityClusterConfigDUnitTest.java
 a4b25f52faa1c19168bef71f02897daa4675f39d 
  
geode-core/src/test/java/org/apache/geode/security/SecurityWithoutClusterConfigDUnitTest.java
 87314ed5b87b409f340ec75c3bcbd7fb838db57c 
  
geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java
 3cafdace9fe10a613a4b16989c87dfdf0bf49a99 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
 ba78fdcf731ad30775357b90766d7757705c 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarter.java 
216acef2036b7a53930a05acf95ef15bef6b5759 
  geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarter.java 
22d3c5671a8faa9f3ee7e6f5051e38e0967c6972 
  
geode-web/src/test/java/org/apache/geode/management/internal/security/GfshCommandsOverHttpSecurityTest.java
 a4162fd9cf6c6e3aaf9215ae108fd9f6d0239180 

Diff: https://reviews.apache.org/r/53241/diff/


Testing
---

precheckin successful


Thanks,

Jinmei Liao

Re: CI failures due to JMX client heartbeat thread

[Jinmei Liao]

Thanks Bruce. Looking at the ConnectToLocatorSSLDUnitTest, we did have a
gfsh there that's not properly closed. I will work with Kirk to close this.
Thanks!

On Thu, Oct 27, 2016 at 11:46 AM, Bruce Schuchardt 
wrote:

> I think the code is in Gfsh.notifyDisconnect().  I think the test that's
> polluting others is ConnectToLocatorSSLDUnitTest.  I had a failure with
> this suspect string in a test that ran just after it. I added a thread dump
> in the @After of that test and see that it's leaving one of these threads
> behind along with some Gfsh Launcher threads.
>
>
> Le 10/27/2016 à 10:54 AM, Kirk Lund a écrit :
>
>> Bruce, can you please point me at the JDK code you think is generating the
>> fatal message? I looked in RMIConnector.java and can't find it in there.
>>
>> Maybe we can fix ordering of tearDown or something else in order to "fix"
>> this message.
>>
>> Thanks,
>> Kirk
>>
>>
>> On Wed, Oct 26, 2016 at 5:00 PM, Dan Smith  wrote:
>>
>> Just looking at a couple of these bugs, these are fatal level errors. Do
>>> you know what is causing them or what affect this might have?
>>>
>>> [fatal 2016/09/29 12:12:03.891 PDT  tid=0x18d]
>>> (tid=397 msgId=39) No longer connected to cc6-co6.gemstone.com[27162].
>>>
>>> -Dan
>>>
>>> On Wed, Oct 26, 2016 at 4:50 PM, Bruce Schuchardt <
>>> bschucha...@pivotal.io>
>>> wrote:
>>>
>>> We have 23 CI failure tickets concerning suspect strings from "JMX client
 heartbeat" threads.  I think we should add this to ExpectedStrings.java

>>> and
>>>
 close these tickets.  The suspect strings are coming from
 javax.management.remote.rmi.RMIConnector and I don't think there's
 anything we can do about them.  Most, if not all, are assigned to

>>> security
>>>
 or JMX components.


 GEODE-1858
 GEODE-1955
 GEODE-1492
 GEODE-1539
 GEODE-1538
 GEODE-1773
 GEODE-1922
 GEODE-1482
 GEODE-1475
 GEODE-1480
 GEODE-1481
 GEODE-1826
 GEODE-1825
 GEODE-1820
 GEODE-1878
 GEODE-1879
 GEODE-1877
 GEODE-1875
 GEODE-1876
 GEODE-1499
 GEODE-1476
 GEODE-1869
 GEODE-1769



>


-- 
Cheers

Jinmei

Re: Review Request 53168: GEODE-2030: security support for SDG

[Jinmei Liao]

> On Oct. 26, 2016, 9:39 p.m., Kirk Lund wrote:
> > geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceTest.java,
> >  line 21
> > <https://reviews.apache.org/r/53168/diff/2/?file=1546442#file1546442line21>
> >
> > This should be:
> > 
> > import static org.assertj.core.api.Assertions.assertThatThrownBy;
> > 
> > Don't use Java6Assertions.

Oh, yeah, thanks for catching this.


- Jinmei


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53168/#review153948
---


On Oct. 26, 2016, 9:12 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53168/
> ---
> 
> (Updated Oct. 26, 2016, 9:12 p.m.)
> 
> 
> Review request for geode, John Blum, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2030: security support for SDG
> 
> * added cacheFactory.setSecurityManager and cacheFactory.setPostProcessor
> * The isIntegragtedSecurity flag is set by checking if the Shiro's 
> securityManger is configured or not.
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/cache/CacheFactory.java 
> b62feac4b2a3ae15e940afb50f71abf61e5eee50 
>   geode-core/src/main/java/org/apache/geode/internal/cache/CacheConfig.java 
> 91ae33304a1e82fa7c02c97edce2318d359220e2 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
>  d9d572c45ac559e611fe2dc86d1028d39d1e592c 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
>  9f4697f6c9f06dbfd671e9e0d80dc52f447e5829 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
>  41b08d510876f95733c96d103b0826726d9a09bb 
>   
> geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceTest.java
>  ee76dfc095c3aeb43a04cee899ba4b434c1e552e 
>   
> geode-core/src/test/java/org/apache/geode/security/CacheFactoryWithSecurityObjectTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53168/diff/
> 
> 
> Testing
> ---
> 
> precheckin successful
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 53168: GEODE-2030: security support for SDG

[Jinmei Liao]

> On Oct. 26, 2016, 6:20 p.m., John Blum wrote:
> > geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java,
> >  line 454
> > <https://reviews.apache.org/r/53168/diff/1/?file=1545181#file1545181line454>
> >
> > Note, this assertion will only apply if assertions were enabled on 
> > startup of the Geode Server JVM Process (i.e. java ... -ea).
> > 
> > That does not appear to be the case...
> > 
> > https://github.com/apache/incubator-geode/blob/rel/v1.0.0-incubating/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java#L1715-L1837
> > 
> > Therefore, it might be better to use an Assert facility, similar to 
> > Spring's Assert class... 
> > http://docs.spring.io/spring/docs/current/javadoc-api/org/springframework/util/Assert.html

I think we don't want to have core depend on Spring libraries. I put in checks 
for null.


- Jinmei


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53168/#review153913
---


On Oct. 26, 2016, 9:12 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53168/
> ---
> 
> (Updated Oct. 26, 2016, 9:12 p.m.)
> 
> 
> Review request for geode, John Blum, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2030: security support for SDG
> 
> * added cacheFactory.setSecurityManager and cacheFactory.setPostProcessor
> * The isIntegragtedSecurity flag is set by checking if the Shiro's 
> securityManger is configured or not.
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/cache/CacheFactory.java 
> b62feac4b2a3ae15e940afb50f71abf61e5eee50 
>   geode-core/src/main/java/org/apache/geode/internal/cache/CacheConfig.java 
> 91ae33304a1e82fa7c02c97edce2318d359220e2 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
>  d9d572c45ac559e611fe2dc86d1028d39d1e592c 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
>  9f4697f6c9f06dbfd671e9e0d80dc52f447e5829 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
>  41b08d510876f95733c96d103b0826726d9a09bb 
>   
> geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceTest.java
>  ee76dfc095c3aeb43a04cee899ba4b434c1e552e 
>   
> geode-core/src/test/java/org/apache/geode/security/CacheFactoryWithSecurityObjectTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53168/diff/
> 
> 
> Testing
> ---
> 
> precheckin successful
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Review Request 53198: GEODE-17: Fix a logical bug that prevents customers to set both Peer and Client authenticator

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53198/
---

Review request for geode, Kevin Duling and Kirk Lund.


Repository: geode


Description
---

GEODE-17: Fix a logical bug that prevents customers to set both peer and Client 
authenticator


Diffs
-

  
geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
 9f4697f6c9f06dbfd671e9e0d80dc52f447e5829 
  
geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceTest.java
 ee76dfc095c3aeb43a04cee899ba4b434c1e552e 

Diff: https://reviews.apache.org/r/53198/diff/


Testing
---

precheckin running


Thanks,

Jinmei Liao

Re: Review Request 53190: [GEODE-2037] Update documentation links

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53190/#review153903
---


Ship it!




Ship It!

- Jinmei Liao


On Oct. 26, 2016, 3:12 p.m., Anthony Baker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53190/
> ---
> 
> (Updated Oct. 26, 2016, 3:12 p.m.)
> 
> 
> Review request for geode, Jinmei Liao, Joey McAllister, and Karen Miller.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Fix README.md, geode-examples/README.md, and the website releases
> page to point to documentation hosted on the geode website
> 
> Update pulse documentation links
> 
> 
> Diffs
> -
> 
>   README.md eee17fc3e975a127418c6721209e20b02a61d38d 
>   geode-examples/README.md 1d4eafb2ccb89c88c3f6ddc8d60475c7c30f7a72 
>   geode-pulse/src/main/webapp/DataBrowser.html 
> 165fa6ff04db294033b90a0909e1df51ca74be41 
>   geode-pulse/src/main/webapp/MemberDetails.html 
> b416926e7249374de66bdb7aaa49a8f1382bb260 
>   geode-pulse/src/main/webapp/QueryStatistics.html 
> 07f61c6ab1a1e5b4467ef185768671c5c0fe1a1b 
>   geode-pulse/src/main/webapp/clusterDetail.html 
> 211e366eac85d9e22f15f3f88e0e05dfc11ea7af 
>   geode-pulse/src/main/webapp/properties/gemfire.properties 
> 17e22598e2e549d8f1bbda6935799ca4e2102d5e 
>   geode-pulse/src/main/webapp/properties/gemfire_en.properties 
> 17e22598e2e549d8f1bbda6935799ca4e2102d5e 
>   geode-pulse/src/main/webapp/regionDetail.html 
> 98404cbf1a95f633b62b17a4b5f9c6795b637c72 
>   geode-site/website/content/releases/index.html 
> 63f685e43b5d9c7ccce487f5c6caeca61f070a8f 
> 
> Diff: https://reviews.apache.org/r/53190/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anthony Baker
> 
>

Review Request 53171: GEODE-17: mark deprecated security configurations

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53171/
---

Review request for geode, Kevin Duling, Kirk Lund, and Swapnil Bawaskar.


Repository: geode


Description
---

GEODE-17: mark deprecated security configurations


Diffs
-

  
geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
 38bec6f4d549bcaad9eb8b0dfe1bf9562919d72a 

Diff: https://reviews.apache.org/r/53171/diff/


Testing
---


Thanks,

Jinmei Liao

Re: Build failed in Jenkins: Geode-nightly #633

[Jinmei Liao]

This failure is fixed in today's checkin.

On Tue, Oct 25, 2016 at 8:49 AM, Apache Jenkins Server <
jenk...@builds.apache.org> wrote:

> See 
>
> Changes:
>
> [jiliao] GEODE-2014: Upgrade Swagger libraries
>
> [jiliao] GEODE-2025: do not use 8080 as the default http-server-port since
> we
>
> [jiliao] GEODE-2014: add the missing license header
>
> [nnag] GEODE-502: Increased the startup timeout for DUnit tests
>
> [bschuchardt] adding spotless line terminator setting for Unix
>
> [jiliao] GEODE-2025: do not use 8080 as the default http-server-port since
> we
>
> [dbarnes] GEODE-2015: Edit links to docs info
>
> --
> [...truncated 554 lines...]
> :geode-core:integrationTest
> :geode-cq:assemble
> :geode-cq:compileTestJavaNote: Some input files use or override a
> deprecated API.
> Note: Recompile with -Xlint:deprecation for details.
> Note: Some input files use unchecked or unsafe operations.
> Note: Recompile with -Xlint:unchecked for details.
>
> :geode-cq:processTestResources
> :geode-cq:testClasses
> :geode-cq:checkMissedTests
> :geode-cq:spotlessJavaCheck
> :geode-cq:spotlessCheck
> :geode-cq:test
> :geode-cq:check
> :geode-cq:build
> :geode-cq:distributedTest
> :geode-cq:flakyTest
> :geode-cq:integrationTest
> :geode-json:assemble
> :geode-json:compileTestJava UP-TO-DATE
> :geode-json:processTestResources UP-TO-DATE
> :geode-json:testClasses UP-TO-DATE
> :geode-json:checkMissedTests UP-TO-DATE
> :geode-json:spotlessJavaCheck
> :geode-json:spotlessCheck
> :geode-json:test UP-TO-DATE
> :geode-json:check
> :geode-json:build
> :geode-json:distributedTest UP-TO-DATE
> :geode-json:flakyTest UP-TO-DATE
> :geode-json:integrationTest UP-TO-DATE
> :geode-junit:javadoc
> :geode-junit:javadocJar
> :geode-junit:sourcesJar
> :geode-junit:signArchives SKIPPED
> :geode-junit:assemble
> :geode-junit:compileTestJava
> :geode-junit:processTestResources UP-TO-DATE
> :geode-junit:testClasses
> :geode-junit:checkMissedTests
> :geode-junit:spotlessJavaCheck
> :geode-junit:spotlessCheck
> :geode-junit:test
> :geode-junit:check
> :geode-junit:build
> :geode-junit:distributedTest
> :geode-junit:flakyTest
> :geode-junit:integrationTest
> :geode-lucene:assemble
> :geode-lucene:compileTestJavaNote: Some input files use or override a
> deprecated API.
> Note: Recompile with -Xlint:deprecation for details.
> Note: Some input files use unchecked or unsafe operations.
> Note: Recompile with -Xlint:unchecked for details.
>
> :geode-lucene:processTestResources
> :geode-lucene:testClasses
> :geode-lucene:checkMissedTests
> :geode-lucene:spotlessJavaCheck
> :geode-lucene:spotlessCheck
> :geode-lucene:test
> :geode-lucene:check
> :geode-lucene:build
> :geode-lucene:distributedTest
> :geode-lucene:flakyTest
> :geode-lucene:integrationTest
> :geode-old-client-support:assemble
> :geode-old-client-support:compileTestJava
> :geode-old-client-support:processTestResources UP-TO-DATE
> :geode-old-client-support:testClasses
> :geode-old-client-support:checkMissedTests
> :geode-old-client-support:spotlessJavaCheck
> :geode-old-client-support:spotlessCheck
> :geode-old-client-support:test
> :geode-old-client-support:check
> :geode-old-client-support:build
> :geode-old-client-support:distributedTest
> :geode-old-client-support:flakyTest
> :geode-old-client-support:integrationTest
> :geode-pulse:assemble
> :geode-pulse:compileTestJavaNote:  job/Geode-nightly/ws/geode-pulse/src/test/java/org/
> apache/geode/tools/pulse/tests/ui/PulseAbstractTest.java> uses or
> overrides a deprecated API.
> Note: Recompile with -Xlint:deprecation for details.
> Note: Some input files use unchecked or unsafe operations.
> Note: Recompile with -Xlint:unchecked for details.
>
> :geode-pulse:processTestResources
> :geode-pulse:testClasses
> :geode-pulse:checkMissedTests
> :geode-pulse:spotlessJavaCheck
> :geode-pulse:spotlessCheck
> :geode-pulse:test
> :geode-pulse:check
> :geode-pulse:build
> :geode-pulse:distributedTest
> :geode-pulse:flakyTest
> :geode-pulse:integrationTest
> :geode-rebalancer:assemble
> :geode-rebalancer:compileTestJava
> :geode-rebalancer:processTestResources UP-TO-DATE
> :geode-rebalancer:testClasses
> :geode-rebalancer:checkMissedTests
> :geode-rebalancer:spotlessJavaCheck
> :geode-rebalancer:spotlessCheck
> :geode-rebalancer:test
> :geode-rebalancer:check
> :geode-rebalancer:build
> :geode-rebalancer:distributedTest
> :geode-rebalancer:flakyTest
> :geode-rebalancer:integrationTest
> :geode-wan:assemble
> :geode-wan:compileTestJavaNote: Some input files use or override a
> deprecated API.
> Note: Recompile with -Xlint:deprecation for details.
> Note: Some input files use unchecked or unsafe operations.
> Note: Recompile with -Xlint:unchecked for details.
>
> :geode-wan:processTestResources
> :geode-wan:testClasses
> :geode-wan:checkMissedTests
> :geode-wan:spotlessJavaCheck
> :geode-wan:spotlessCheck
> :geode-wan:test
> 

Review Request 53168: GEODE-2030: security support for SDG

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53168/
---

Review request for geode, John Blum, Kevin Duling, and Kirk Lund.


Repository: geode


Description
---

GEODE-2030: security support for SDG

* added cacheFactory.setSecurityManager and cacheFactory.setPostProcessor
* The isIntegragtedSecurity flag is set by checking if the Shiro's 
securityManger is configured or not.


Diffs
-

  geode-core/src/main/java/org/apache/geode/cache/CacheFactory.java 
b62feac4b2a3ae15e940afb50f71abf61e5eee50 
  geode-core/src/main/java/org/apache/geode/internal/cache/CacheConfig.java 
91ae33304a1e82fa7c02c97edce2318d359220e2 
  
geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java 
d9d572c45ac559e611fe2dc86d1028d39d1e592c 
  
geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
 9f4697f6c9f06dbfd671e9e0d80dc52f447e5829 
  
geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
 41b08d510876f95733c96d103b0826726d9a09bb 
  
geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceTest.java
 ee76dfc095c3aeb43a04cee899ba4b434c1e552e 
  
geode-core/src/test/java/org/apache/geode/security/CacheFactoryWithSecurityObjectTest.java
 PRE-CREATION 

Diff: https://reviews.apache.org/r/53168/diff/


Testing
---

precheckin successful


Thanks,

Jinmei Liao

Review Request 53093: GEODE-2025: do not use 8080 as the default http-server-port since we defined default in DistributionConfig

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53093/
---

Review request for geode.


Repository: geode


Description
---

GEODE-2025: do not use 8080 as the default http-server-port since we defined 
default in DistributionConfig

When we pulled in a pull request in Auguest, we didn't notice that the default 
http-service-port it's using is 8080. In DistributionConfig, we defined it to 
be 7070. This would cause a discrepency between the default http port we are 
using between locators and servers. change this to 7070 now.


Diffs
-

  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/GeodeRestClient.java
 c83cebb745ff931e28b81781a53952baf9465bd1 
  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java
 5f66f3bd48055cd0b65726710d36c4fac1d30b40 
  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestServersJUnitTest.java
 PRE-CREATION 
  geode-core/src/main/java/org/apache/geode/cache/server/CacheServer.java 
910f34d448a27497a541793d8a6c260e56970d00 
  
geode-core/src/main/java/org/apache/geode/internal/cache/execute/util/FindRestEnabledServersFunction.java
 793f73c8cae62188815cf29fb98da8148cb140a8 
  geode-core/src/main/java/org/apache/geode/management/internal/RestAgent.java 
6e7ba7ddd9c21532c1c4978b6af81012a482b782 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
 892a92d688a88de5742e70f28ff5442358a3b218 

Diff: https://reviews.apache.org/r/53093/diff/


Testing
---


Thanks,

Jinmei Liao

Re: Review Request 53079: GEODE-2020: for rest api get request, use utf-8 as response encoding.

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53079/
---

(Updated Oct. 21, 2016, 2:29 p.m.)


Review request for geode, Kevin Duling and Kirk Lund.


Repository: geode


Description
---

* add more test assertions.
* fix legacy tests


Diffs
-

  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/GeodeRestClient.java
 PRE-CREATION 
  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java
 6e91894c2c50e21c1fa250330119a7013f6b0fa5 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/CommonCrudController.java
 30c8b3aa160648cae2ae2176e746ff987aba3aec 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/FunctionAccessController.java
 e1ea1ade2e8633f983f60fcf3cb9b789cd9dc2f4 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java
 ebb8ccc08fe1766d278eaeaf053365ad0e9afcfb 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/QueryAccessController.java
 d13c99c11151355f3595718b15505db779d4161e 

Diff: https://reviews.apache.org/r/53079/diff/


Testing (updated)
---

precheckin successful


Thanks,

Jinmei Liao

Re: Review Request 53079: GEODE-2020: for rest api get request, use utf-8 as response encoding.

[Jinmei Liao]

> On Oct. 20, 2016, 11:15 p.m., Kevin Duling wrote:
> > We've just stomped all over each other in RestSecurityIntegrationTest.  
> > Some time back, you'd refactored RestSecurityEndpointsDUnitTest to change 
> > it from a DUnit to an Integration test.  But in that change, you removed 
> > the abstract base class I'd placed doGet, doPut, etc., in to.
> > 
> > With the test I need for the new Swagger, I re-introduced the base class to 
> > avoid a lot of code duplication.  As a result, your changes are 
> > incompatible with the changes I have in GEODE-2014.
> > 
> > The changes aren't large, but I wanted to point out the merge conflict 
> > we're going to have.

I pulled all that utility methods into a GeodeRestClient object. It's easier to 
share code than an abstract class. I am gonna handle your pull request anyway. 
I can do the merge at that time.


- Jinmei


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53079/#review153469
-------


On Oct. 20, 2016, 10:31 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53079/
> ---
> 
> (Updated Oct. 20, 2016, 10:31 p.m.)
> 
> 
> Review request for geode, Kevin Duling and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * add more test assertions.
> * fix legacy tests
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/GeodeRestClient.java
>  PRE-CREATION 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java
>  6e91894c2c50e21c1fa250330119a7013f6b0fa5 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/CommonCrudController.java
>  30c8b3aa160648cae2ae2176e746ff987aba3aec 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/FunctionAccessController.java
>  e1ea1ade2e8633f983f60fcf3cb9b789cd9dc2f4 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java
>  ebb8ccc08fe1766d278eaeaf053365ad0e9afcfb 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/QueryAccessController.java
>  d13c99c11151355f3595718b15505db779d4161e 
> 
> Diff: https://reviews.apache.org/r/53079/diff/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 53079: GEODE-2020: for rest api get request, use utf-8 as response encoding.

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53079/
---

(Updated Oct. 20, 2016, 10:31 p.m.)


Review request for geode, Kevin Duling and Kirk Lund.


Repository: geode


Description
---

* add more test assertions.
* fix legacy tests


Diffs
-

  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/GeodeRestClient.java
 PRE-CREATION 
  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java
 6e91894c2c50e21c1fa250330119a7013f6b0fa5 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/CommonCrudController.java
 30c8b3aa160648cae2ae2176e746ff987aba3aec 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/FunctionAccessController.java
 e1ea1ade2e8633f983f60fcf3cb9b789cd9dc2f4 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java
 ebb8ccc08fe1766d278eaeaf053365ad0e9afcfb 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/QueryAccessController.java
 d13c99c11151355f3595718b15505db779d4161e 

Diff: https://reviews.apache.org/r/53079/diff/


Testing (updated)
---

precheckin running


Thanks,

Jinmei Liao

Review Request 53079: GEODE-2020: for rest api get request, use utf-8 as response encoding.

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53079/
---

Review request for geode.


Repository: geode


Description
---

* add more test assertions.
* fix legacy tests


Diffs
-

  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/GeodeRestClient.java
 PRE-CREATION 
  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java
 6e91894c2c50e21c1fa250330119a7013f6b0fa5 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/CommonCrudController.java
 30c8b3aa160648cae2ae2176e746ff987aba3aec 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/FunctionAccessController.java
 e1ea1ade2e8633f983f60fcf3cb9b789cd9dc2f4 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java
 ebb8ccc08fe1766d278eaeaf053365ad0e9afcfb 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/QueryAccessController.java
 d13c99c11151355f3595718b15505db779d4161e 

Diff: https://reviews.apache.org/r/53079/diff/


Testing
---


Thanks,

Jinmei Liao

Re: Review Request 52994: GEODE-1959: prompt for password when starting a server if username is specified

[Jinmei Liao]

> On Oct. 18, 2016, 11:28 p.m., Kirk Lund wrote:
> > I think you may need to find some API to ask GFSH if it's running in 
> > interactive mode or script mode. Then if it's in script mode, it should 
> > fail instead of prompting. John Blum might be a good person to ask if they 
> > know how to check interactive vs script mode. He would be a good reviewer 
> > for this changeset in general.

I believe the gfsh.readPassword and gfsh.readText already does that. I just 
moved these two private method in ShellCommands.java into Gfsh and make that 
public so that my LauncherLifecycleCommands can use it as well. I will add John 
to the reviewer as well.


- Jinmei


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52994/#review153212
---


On Oct. 19, 2016, 12:11 a.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52994/
> ---
> 
> (Updated Oct. 19, 2016, 12:11 a.m.)
> 
> 
> Review request for geode, John Blum, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1959: prompt for password when starting a server if username is 
> specified
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsDUnitTest.java
>  490e309dad7ee5100a2c39fc8ec3b13bc9c30737 
>   geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java 
> a3d3845bf218cd04500a336e65d6143277433f7f 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  89a9a37d917eb29eddd12676621417c5e3eadd25 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
>  4ffe08271d0e06919f45769f200b71ef0978d879 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java
>  792a8abed97dcff8c15f678a4cd12e0d1656568d 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
>  51887cf500f6aed00f79c10b22f708b03a13d178 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
>  467682d9b931e0bdd03c8dd3a739b686a6ace1c3 
>   
> geode-core/src/test/resources/org/apache/geode/management/internal/cli/commands/golden-help-offline.properties
>  28083f3058655b46c027c23dd64225f449ddb84e 
> 
> Diff: https://reviews.apache.org/r/52994/diff/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 52994: GEODE-1959: prompt for password when starting a server if username is specified

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52994/
---

(Updated Oct. 18, 2016, 11:09 p.m.)


Review request for geode, Kevin Duling and Kirk Lund.


Changes
---

making error messages more precise


Repository: geode


Description
---

GEODE-1959: prompt for password when starting a server if username is specified


Diffs (updated)
-

  
geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsDUnitTest.java
 490e309dad7ee5100a2c39fc8ec3b13bc9c30737 
  geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java 
a3d3845bf218cd04500a336e65d6143277433f7f 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
 89a9a37d917eb29eddd12676621417c5e3eadd25 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
 4ffe08271d0e06919f45769f200b71ef0978d879 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java
 792a8abed97dcff8c15f678a4cd12e0d1656568d 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
 51887cf500f6aed00f79c10b22f708b03a13d178 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
 467682d9b931e0bdd03c8dd3a739b686a6ace1c3 
  
geode-core/src/test/resources/org/apache/geode/management/internal/cli/commands/golden-help-offline.properties
 28083f3058655b46c027c23dd64225f449ddb84e 

Diff: https://reviews.apache.org/r/52994/diff/


Testing
---

precheckin running


Thanks,

Jinmei Liao

Review Request 52994: GEODE-1959: prompt for password when starting a server if username is specified

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52994/
---

Review request for geode, Kevin Duling and Kirk Lund.


Repository: geode


Description
---

GEODE-1959: prompt for password when starting a server if username is specified


Diffs
-

  
geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsDUnitTest.java
 490e309dad7ee5100a2c39fc8ec3b13bc9c30737 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
 4ffe08271d0e06919f45769f200b71ef0978d879 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java
 792a8abed97dcff8c15f678a4cd12e0d1656568d 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
 51887cf500f6aed00f79c10b22f708b03a13d178 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
 467682d9b931e0bdd03c8dd3a739b686a6ace1c3 
  
geode-core/src/test/resources/org/apache/geode/management/internal/cli/commands/golden-help-offline.properties
 28083f3058655b46c027c23dd64225f449ddb84e 

Diff: https://reviews.apache.org/r/52994/diff/


Testing
---

precheckin running


Thanks,

Jinmei Liao

Re: Review Request 52963: GEODE-388: Deprecating DynamicRegionFactory

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52963/#review152995
---


Ship it!




Ship It!

- Jinmei Liao


On Oct. 18, 2016, 12:06 a.m., Dan Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52963/
> ---
> 
> (Updated Oct. 18, 2016, 12:06 a.m.)
> 
> 
> Review request for geode and Darrel Schneider.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Marking DynamicRegionFactory as deprecated.
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/cache/DynamicRegionFactory.java 
> 3cfa73b840785cab94d4ca54aac1338304a20f30 
> 
> Diff: https://reviews.apache.org/r/52963/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Smith
> 
>

Re: Review Request 52936: GEODE-2006: add FlakyTest category to testSelectCommand

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52936/#review152931
---


Ship it!




Ship It!

- Jinmei Liao


On Oct. 17, 2016, 5:59 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52936/
> ---
> 
> (Updated Oct. 17, 2016, 5:59 p.m.)
> 
> 
> Review request for geode, Jinmei Liao and Kevin Duling.
> 
> 
> Bugs: GEODE-2006
> https://issues.apache.org/jira/browse/GEODE-2006
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2006: add FlakyTest category to testSelectCommand
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/GemfireDataCommandsDUnitTest.java
>  5417ccb 
> 
> Diff: https://reviews.apache.org/r/52936/diff/
> 
> 
> Testing
> ---
> 
> build, GemfireDataCommandsDUnitTest
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Re: Review Request 52949: GEODE-2009: apply FlakyTest category to testCreateAlterDestroyUpdatesSharedConfig

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52949/#review152930
---


Ship it!




Ship It!

- Jinmei Liao


On Oct. 17, 2016, 7:18 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52949/
> ---
> 
> (Updated Oct. 17, 2016, 7:18 p.m.)
> 
> 
> Review request for geode, Jinmei Liao and Kevin Duling.
> 
> 
> Bugs: GEODE-2009
> https://issues.apache.org/jira/browse/GEODE-2009
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2009: apply FlakyTest category to 
> testCreateAlterDestroyUpdatesSharedConfig
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommandsDUnitTest.java
>  5fa06d9 
> 
> Diff: https://reviews.apache.org/r/52949/diff/
> 
> 
> Testing
> ---
> 
> build, CreateAlterDestroyRegionCommandsDUnitTest
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Re: Hosting the docs (was Re: [VOTE] Release Apache Geode (incubating) 1.0.0-incubating - RC2)

[Jinmei Liao]

+1

On Mon, Oct 17, 2016 at 12:05 PM, Dan Smith  wrote:

> +1 - I was going to ask about this on the release thread. Let's put them up
> on geode.apache.org!
>
> -Dan
>
> On Mon, Oct 17, 2016 at 11:33 AM, Swapnil Bawaskar 
> wrote:
>
> > +1, It would be great to have this by the time we put the vote out on
> > general@incubator
> >
> > On Mon, Oct 17, 2016 at 10:59 AM, Joey McAllister <
> jmcallis...@pivotal.io>
> > wrote:
> >
> > > Only a very enthusiastic +1. :)
> > >
> > > On Mon, Oct 17, 2016 at 10:57 AM Anthony Baker 
> > wrote:
> > >
> > > > Since the geode docs have now been merged to the develop branch,
> let’s
> > > > start hosting them on http://geode.apache.org.  Thoughts?
> > > >
> > > > Anthony
> > > >
> > > >
> > > > On Oct 15, 2016, at 5:51 PM, Swapnil Bawaskar 
> > > > wrote:
> > > >
> > > >
> > > > The documentation on how to install and use Apache Geode are hosted
> > > > on pivotal.io:
> > > >   http://geode.docs.pivotal.io
> > > >
> > > >
> > > >
> > >
> >
>



-- 
Cheers

Jinmei

Re: Review Request 52935: GEODE-2005: fix javadoc warning

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52935/#review152911
---


Ship it!




Ship It!

- Jinmei Liao


On Oct. 17, 2016, 5:44 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52935/
> ---
> 
> (Updated Oct. 17, 2016, 5:44 p.m.)
> 
> 
> Review request for geode, Jinmei Liao and Kevin Duling.
> 
> 
> Bugs: GEODE-2005
> https://issues.apache.org/jira/browse/GEODE-2005
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Fix javadoc warning.
> Line up @param and @return tags.
> Minor cleanup of javadocs.
> Delete extra blank lines.
> 
> 
> Diffs
> -
> 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/CommonCrudController.java
>  2bcb31b 
> 
> Diff: https://reviews.apache.org/r/52935/diff/
> 
> 
> Testing
> ---
> 
> javadoc
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Re: [VOTE] Release Apache Geode (incubating) 1.0.0-incubating - RC2

[Jinmei Liao]

+0

This candidate does not include the fix for GEODE-2004 and GEODE-1883. It's
not a must fix though.

On Sun, Oct 16, 2016 at 6:58 PM, Anthony Baker  wrote:

> +1
>
> * Verified sha’s
> * Verified signatures
> * Verified tag signature
> * Build and run from src distro
> * Checked src distro for binaries
> * Run some examples from mvn repo
> * Reviewed LICENSE and NOTICE
>
> Anthony
>
> > On Oct 15, 2016, at 5:51 PM, Swapnil Bawaskar 
> wrote:
> >
> > All,
> >
> > This is the second release candidate for Apache Geode, version
> > 1.0.0-incubating. I discarded the first release candidate since my pgp
> > key was missing from the KEYS file.
> > Thanks to all the community members to drive towards this milestone!
> >
> > It fixes the following issues:
> >
> >https://issues.apache.org/jira/secure/ReleaseNote.jspa?
> projectId=12318420=12332343
> >
> > *** Please download, test and vote by Wednesday, October 19, 0800 hrs
> > US Pacific.
> >
> > Note that we are voting upon the source (tag):
> >   rel/1.0.0-incubating.RC2
> >  geode.git;a=tag;h=refs/tags/rel/v1.0.0-incubating.RC2>
> >
> > Commit ID: 280a407c59a89401d5d87d6e6aeda1c975870753
> >  geode.git;a=commit;h=280a407c59a89401d5d87d6e6aeda1c975870753>
> >
> > Source and binary files:
> >   https://dist.apache.org/repos/dist/dev/incubator/geode/1.0.
> 0-incubating.RC2/
> >
> > The documentation on how to install and use Apache Geode are hosted
> > on pivotal.io:
> >   http://geode.docs.pivotal.io
> >
> > Maven staging repo:
> >   https://repository.apache.org/content/repositories/
> orgapachegeode-1014/
> >
> > Geode's KEYS file containing PGP keys we use to sign the release:
> >   https://github.com/apache/incubator-geode/blob/release/
> 1.0.0-incubating/KEYS
> >
> > Release Signed with Key: pub   4096R/18F902DB 2016-04-07
> > Fingerprint: E1B1 ABE3 4753 E7BA 8097  4285 8F8F 2BCC 18F9 02DB
> >
> > Thanks,
> >
> > Swapnil.
>
>


-- 
Cheers

Jinmei

Review Request 52931: GEODE-1993: allow LocatorServerStartupRule to save server's ports as well.

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52931/
---

Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.


Repository: geode


Description
---

* added more tetss


Diffs
-

  
geode-core/src/test/java/org/apache/geode/security/ClusterConfigWithoutSecurityDUnitTest.java
 72dbd1aff32c718c1e6f72b940ade523c9cd8847 
  
geode-core/src/test/java/org/apache/geode/security/PeerAuthenticatorDUnitTest.java
 bb147c7a24868adf0aaa1e860ca6114ffa032afc 
  
geode-core/src/test/java/org/apache/geode/security/PeerSecurityWithEmbeddedLocatorDUnitTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/security/SecurityClusterConfigDUnitTest.java
 5364c910a4c4e9d24de8be26b63a62428f48d0a7 
  
geode-core/src/test/java/org/apache/geode/security/SecurityWithoutClusterConfigDUnitTest.java
 d3ed823887c15ca06aba76e20a964c25e5dee8b4 
  
geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java
 f6928bfa0db529c38b521d4bc72d013272981e1a 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
 71894c8a27a8768db2e7e99b1a2b981253e4ec67 
  
geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java
 bcc5ab3bca842d741bced5c59117472c77659ba6 

Diff: https://reviews.apache.org/r/52931/diff/


Testing
---


Thanks,

Jinmei Liao

Re: Review Request 52889: GEODE-1993: refactor tests to use rules rather than abstract classes

[Jinmei Liao]

 cf0df1b66bfa25bec3d110fc398108a6fbca8d4e 
  
geode-core/src/test/java/org/apache/geode/security/PostProcessorDUnitTest.java 
a7cdb0fbfed65a5af1249f15f698efb0454f064a 
  
geode-core/src/test/java/org/apache/geode/security/SecurityClusterConfigDUnitTest.java
 54c02f787f923ff7305bdc2ede81044fac446944 
  
geode-core/src/test/java/org/apache/geode/security/SecurityWithoutClusterConfigDUnitTest.java
 d5f868634e7b4f93a9a5d6da44ef0e2c0c2729ae 
  
geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java
 953cdb746beb220b148555952c9d27a2389e3a01 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerConfigurationRule.java
 7f52ce1d54dd71e9a1c53323d3388e3ebca82ecd 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarter.java 
PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarter.java 
PRE-CREATION 
  geode-cq/src/test/java/org/apache/geode/security/CQClientAuthDunitTest.java 
2386af1ec47e8f75350304d597128ef8aa63a3b7 
  
geode-cq/src/test/java/org/apache/geode/security/CQPDXPostProcessorDUnitTest.java
 12f08ec49c42bec6aaf90b676d09d53b0eee8fd9 
  
geode-cq/src/test/java/org/apache/geode/security/CQPostProcessorDunitTest.java 
e2b555a416d1a1896898959f9207477237f2af36 
  
geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java
 30532b944cf6d171a26cd7800acfd30e1d151f67 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/CommonCrudController.java
 35d8fb4629a3b706cb30c5b90e1a1d59ca0aa556 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java
 3003b3d673c0ee5f075b93e204a65660ff3e568e 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/RestSecurityService.java
 3d09f096a500e2533bb3e7e4ba330bffac209c35 

Diff: https://reviews.apache.org/r/52889/diff/


Testing
---

precheckin successful


Thanks,

Jinmei Liao

Re: Review Request 52889: GEODE-1993: refactor tests to use rules rather than abstract classes

[Jinmei Liao]

 d2a98875d96ff55f6b6eec11c8695041ce8cd309 
  
geode-core/src/test/java/org/apache/geode/security/PDXPostProcessorDUnitTest.java
 cf0df1b66bfa25bec3d110fc398108a6fbca8d4e 
  
geode-core/src/test/java/org/apache/geode/security/PostProcessorDUnitTest.java 
a7cdb0fbfed65a5af1249f15f698efb0454f064a 
  
geode-core/src/test/java/org/apache/geode/security/SecurityClusterConfigDUnitTest.java
 54c02f787f923ff7305bdc2ede81044fac446944 
  
geode-core/src/test/java/org/apache/geode/security/SecurityWithoutClusterConfigDUnitTest.java
 d5f868634e7b4f93a9a5d6da44ef0e2c0c2729ae 
  
geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java
 953cdb746beb220b148555952c9d27a2389e3a01 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerConfigurationRule.java
 7f52ce1d54dd71e9a1c53323d3388e3ebca82ecd 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarter.java 
PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarter.java 
PRE-CREATION 
  geode-cq/src/test/java/org/apache/geode/security/CQClientAuthDunitTest.java 
2386af1ec47e8f75350304d597128ef8aa63a3b7 
  
geode-cq/src/test/java/org/apache/geode/security/CQPDXPostProcessorDUnitTest.java
 12f08ec49c42bec6aaf90b676d09d53b0eee8fd9 
  
geode-cq/src/test/java/org/apache/geode/security/CQPostProcessorDunitTest.java 
e2b555a416d1a1896898959f9207477237f2af36 
  
geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java
 30532b944cf6d171a26cd7800acfd30e1d151f67 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/CommonCrudController.java
 35d8fb4629a3b706cb30c5b90e1a1d59ca0aa556 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java
 3003b3d673c0ee5f075b93e204a65660ff3e568e 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/RestSecurityService.java
 3d09f096a500e2533bb3e7e4ba330bffac209c35 

Diff: https://reviews.apache.org/r/52889/diff/


Testing
---

precheckin successful


Thanks,

Jinmei Liao

Re: Review Request 52889: GEODE-1993: refactor tests to use rules rather than abstract classes

[Jinmei Liao]

> On Oct. 14, 2016, 5:21 p.m., Jared Stewart wrote:
> > Many usages of ServerStarter have to specify 
> > "org/apache/geode/management/internal/security/clientServer.json".  I think 
> > it would be convenient to have some static factory methods like 
> > ServerStarter.startWithSecurity() that specify a default set of Properties 
> > like SampleSecurityManager and 
> > "org/apache/geode/management/internal/security/clientServer.json". 
> > 
> > The same addition would be useful in CacheServerStartupRule.  It would be 
> > convenient to not have to pass in 
> > "org/apache/geode/management/internal/security/cacheServer.json".
> > 
> > RestSecurityService has authorize methods that show up as unused in 
> > Intellij.  I know from looking at the other changes that they are used by 
> > @PreAuthorized annotations in the CrudControllers, but it might be nice to 
> > add comments to these methods explaining where they're used so that people 
> > in the future don't think they can be deleted.

The thing is, that json file is very test specific. Each test can use a 
different json file to change the username/password it's using and permissions 
granted to that user. We started the tests using this SampleSecurityManager 
which unfortunatly requires the json file. I would hope all our later tests 
would use SimpleSecurityManager instead to get rid this.


- Jinmei


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52889/#review152694
---


On Oct. 14, 2016, 5:25 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52889/
> ---
> 
> (Updated Oct. 14, 2016, 5:25 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * created ServerStarter and LocatorStarter in the rule package
> * refacterred LocatorServerConfigurationRule
> * refactor tests to use these rules
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java
>  59e00c8bd0a7f2759f579abf99a87fcd98b42a06 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
>  79b70f875f74c15eb041e9d1cbd5b1f8ceeda899 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/AccessControlMBeanJUnitTest.java
>  c22fff3655131cf908a54289b66faf84311c5c69 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanAuthenticationJUnitTest.java
>  388094840fc587e662d231783fd5c7c8faf7b485 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanAuthorizationJUnitTest.java
>  7bbfbcc9691000601cf6bf4dd0be7c7394cf3fcf 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanShiroJUnitTest.java
>  721a4312456e0f1eaf41a6c1f41016cfe56450e4 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerStartupRule.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CliCommandsSecurityTest.java
>  84155a9d82292dc70c6412908a1b57a09c1aea98 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/DataCommandsSecurityTest.java
>  0084cb8f5471ce7ecab086c955a842766d6ed790 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/DiskStoreMXBeanSecurityJUnitTest.java
>  750ce2ac76ecd0860f5f678a22615697c9ea5009 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GatewayReceiverMBeanSecurityTest.java
>  b64a6f7b1c63eb41f5823b4971d5041431748db9 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GatewaySenderMBeanSecurityTest.java
>  9acf8dbd172cca27ba4da942b3d5c1bd0368ff83 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java
>  34fd5a994c5a69d64398de3cb867892a30827e2c 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java
>  def17920b198e88f358dfcf4025f4a3eee6fd0df 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshShellConnectionRule.java
>  4d1bae904c4964ca2be713fd8450f2f5f6db2552 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/se

Re: Review Request 52889: GEODE-1993: refactor tests to use rules rather than abstract classes

[Jinmei Liao]

> On Oct. 14, 2016, 5:12 p.m., Kirk Lund wrote:
> > Consider having *Rule in the name of the new rules.
> > 
> > Fix issues and Ship It!

I struggled with this too. The thing is: LocatorStarter and ServerStarter can 
be used not as a rule. they don't even have a before() method. See 
LocatorServerStartupRule which simply utilizes these two classes and handles 
the closing in its own after method.


- Jinmei


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52889/#review152695
---


On Oct. 14, 2016, 5:25 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52889/
> ---
> 
> (Updated Oct. 14, 2016, 5:25 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * created ServerStarter and LocatorStarter in the rule package
> * refacterred LocatorServerConfigurationRule
> * refactor tests to use these rules
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java
>  59e00c8bd0a7f2759f579abf99a87fcd98b42a06 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
>  79b70f875f74c15eb041e9d1cbd5b1f8ceeda899 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/AccessControlMBeanJUnitTest.java
>  c22fff3655131cf908a54289b66faf84311c5c69 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanAuthenticationJUnitTest.java
>  388094840fc587e662d231783fd5c7c8faf7b485 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanAuthorizationJUnitTest.java
>  7bbfbcc9691000601cf6bf4dd0be7c7394cf3fcf 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanShiroJUnitTest.java
>  721a4312456e0f1eaf41a6c1f41016cfe56450e4 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerStartupRule.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CliCommandsSecurityTest.java
>  84155a9d82292dc70c6412908a1b57a09c1aea98 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/DataCommandsSecurityTest.java
>  0084cb8f5471ce7ecab086c955a842766d6ed790 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/DiskStoreMXBeanSecurityJUnitTest.java
>  750ce2ac76ecd0860f5f678a22615697c9ea5009 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GatewayReceiverMBeanSecurityTest.java
>  b64a6f7b1c63eb41f5823b4971d5041431748db9 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GatewaySenderMBeanSecurityTest.java
>  9acf8dbd172cca27ba4da942b3d5c1bd0368ff83 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java
>  34fd5a994c5a69d64398de3cb867892a30827e2c 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java
>  def17920b198e88f358dfcf4025f4a3eee6fd0df 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshShellConnectionRule.java
>  4d1bae904c4964ca2be713fd8450f2f5f6db2552 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/JMXConnectionConfiguration.java
>  4f57baa85e3b5cd084ffd33c61f7741d20b0fdc9 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/JavaRmiServerNameTest.java
>  c544e6fe0f8d3c1975775979d1354c4adc7cf87a 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/JsonAuthorizationCacheStartRule.java
>  136319c06d9bd1e46b452219bf2894cf969fc1f1 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/LockServiceMBeanAuthorizationJUnitTest.java
>  1377fb643d035ad911ee5ceb80b87dfd72855428 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/MBeanSecurityJUnitTest.java
>  4beff0bd55d69601266a0a856c8dcb80ffd4e1f7 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/MBeanServerConnectionRule.java
>  92430327e999af627e88956fc24aa1592e9a000d 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/ManagerMBeanAuthorizationJUnitTest.java
>  873b6491f6703378d87383d96dc35e299b19a3fa 
>   
&

Re: Review Request 52889: GEODE-1993: refactor tests to use rules rather than abstract classes

[Jinmei Liao]

> On Oct. 14, 2016, 5:12 p.m., Kirk Lund wrote:
> > geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java,
> >  line 66
> > <https://reviews.apache.org/r/52889/diff/1/?file=1538108#file1538108line66>
> >
> > This is still a DistributedTest if it uses the dunit VMs. This uses a 
> > dunit rule so it probably still uses dunit VMs.

This test uses the ServerStarter rule now. I just pushed another set of change 
that adds more javadoc to the ServerStarter and LocatorStarter rule. Basically 
these twos rules starts up server/locator in the current VM. It's not using 
dunit VMs. This makes this test runs a lot faster. But you are right, the test 
itself should not be called a DunitTest anymore. I'll fix that.


- Jinmei


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52889/#review152695
-------


On Oct. 14, 2016, 5:25 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52889/
> ---
> 
> (Updated Oct. 14, 2016, 5:25 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * created ServerStarter and LocatorStarter in the rule package
> * refacterred LocatorServerConfigurationRule
> * refactor tests to use these rules
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java
>  59e00c8bd0a7f2759f579abf99a87fcd98b42a06 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
>  79b70f875f74c15eb041e9d1cbd5b1f8ceeda899 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/AccessControlMBeanJUnitTest.java
>  c22fff3655131cf908a54289b66faf84311c5c69 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanAuthenticationJUnitTest.java
>  388094840fc587e662d231783fd5c7c8faf7b485 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanAuthorizationJUnitTest.java
>  7bbfbcc9691000601cf6bf4dd0be7c7394cf3fcf 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanShiroJUnitTest.java
>  721a4312456e0f1eaf41a6c1f41016cfe56450e4 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerStartupRule.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CliCommandsSecurityTest.java
>  84155a9d82292dc70c6412908a1b57a09c1aea98 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/DataCommandsSecurityTest.java
>  0084cb8f5471ce7ecab086c955a842766d6ed790 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/DiskStoreMXBeanSecurityJUnitTest.java
>  750ce2ac76ecd0860f5f678a22615697c9ea5009 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GatewayReceiverMBeanSecurityTest.java
>  b64a6f7b1c63eb41f5823b4971d5041431748db9 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GatewaySenderMBeanSecurityTest.java
>  9acf8dbd172cca27ba4da942b3d5c1bd0368ff83 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java
>  34fd5a994c5a69d64398de3cb867892a30827e2c 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java
>  def17920b198e88f358dfcf4025f4a3eee6fd0df 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshShellConnectionRule.java
>  4d1bae904c4964ca2be713fd8450f2f5f6db2552 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/JMXConnectionConfiguration.java
>  4f57baa85e3b5cd084ffd33c61f7741d20b0fdc9 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/JavaRmiServerNameTest.java
>  c544e6fe0f8d3c1975775979d1354c4adc7cf87a 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/JsonAuthorizationCacheStartRule.java
>  136319c06d9bd1e46b452219bf2894cf969fc1f1 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/LockServiceMBeanAuthorizationJUnitTest.java
>  1377fb643d035ad911ee5ceb80b87dfd72855428 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/MBeanSecurityJUnitTest.java
>  4beff0bd55

Re: Review Request 52889: GEODE-1993: refactor tests to use rules rather than abstract classes

[Jinmei Liao]

/PostProcessorDUnitTest.java 
a7cdb0fbfed65a5af1249f15f698efb0454f064a 
  
geode-core/src/test/java/org/apache/geode/security/SecurityClusterConfigDUnitTest.java
 54c02f787f923ff7305bdc2ede81044fac446944 
  
geode-core/src/test/java/org/apache/geode/security/SecurityWithoutClusterConfigDUnitTest.java
 d5f868634e7b4f93a9a5d6da44ef0e2c0c2729ae 
  
geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java
 953cdb746beb220b148555952c9d27a2389e3a01 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerConfigurationRule.java
 7f52ce1d54dd71e9a1c53323d3388e3ebca82ecd 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarter.java 
PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarter.java 
PRE-CREATION 
  geode-cq/src/test/java/org/apache/geode/security/CQClientAuthDunitTest.java 
2386af1ec47e8f75350304d597128ef8aa63a3b7 
  
geode-cq/src/test/java/org/apache/geode/security/CQPDXPostProcessorDUnitTest.java
 12f08ec49c42bec6aaf90b676d09d53b0eee8fd9 
  
geode-cq/src/test/java/org/apache/geode/security/CQPostProcessorDunitTest.java 
e2b555a416d1a1896898959f9207477237f2af36 
  
geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java
 30532b944cf6d171a26cd7800acfd30e1d151f67 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/CommonCrudController.java
 35d8fb4629a3b706cb30c5b90e1a1d59ca0aa556 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java
 3003b3d673c0ee5f075b93e204a65660ff3e568e 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/RestSecurityService.java
 3d09f096a500e2533bb3e7e4ba330bffac209c35 

Diff: https://reviews.apache.org/r/52889/diff/


Testing
---

precheckin successful


Thanks,

Jinmei Liao

Review Request 52889: GEODE-1993: refactor tests to use rules rather than abstract classes

[Jinmei Liao]

/security/SecurityClusterConfigDUnitTest.java
 54c02f787f923ff7305bdc2ede81044fac446944 
  
geode-core/src/test/java/org/apache/geode/security/SecurityWithoutClusterConfigDUnitTest.java
 d5f868634e7b4f93a9a5d6da44ef0e2c0c2729ae 
  
geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java
 953cdb746beb220b148555952c9d27a2389e3a01 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerConfigurationRule.java
 7f52ce1d54dd71e9a1c53323d3388e3ebca82ecd 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarter.java 
PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarter.java 
PRE-CREATION 
  geode-cq/src/test/java/org/apache/geode/security/CQClientAuthDunitTest.java 
2386af1ec47e8f75350304d597128ef8aa63a3b7 
  
geode-cq/src/test/java/org/apache/geode/security/CQPDXPostProcessorDUnitTest.java
 12f08ec49c42bec6aaf90b676d09d53b0eee8fd9 
  
geode-cq/src/test/java/org/apache/geode/security/CQPostProcessorDunitTest.java 
e2b555a416d1a1896898959f9207477237f2af36 
  
geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java
 30532b944cf6d171a26cd7800acfd30e1d151f67 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/CommonCrudController.java
 35d8fb4629a3b706cb30c5b90e1a1d59ca0aa556 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java
 3003b3d673c0ee5f075b93e204a65660ff3e568e 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/RestSecurityService.java
 3d09f096a500e2533bb3e7e4ba330bffac209c35 

Diff: https://reviews.apache.org/r/52889/diff/


Testing
---

precheckin successful


Thanks,

Jinmei Liao

Re: Starting the server, prompt for username/password

[Jinmei Liao]

member can provide an auth-init method to get those credentials instead of
explicitly specify username/password as well. So you will need to check for
that as well.

On Thu, Oct 13, 2016 at 3:20 PM, Kevin Duling  wrote:

> security-username and security-password appear to be the properties most
> commonly set/checked.  I could base prompting off of their existence and
> null value.
>
> On Thu, Oct 13, 2016 at 2:55 PM, Kirk Lund  wrote:
>
> > I think we should avoid using an exception for normal flow control. I'd
> > provide a link to Ward Cunningham's wiki but his wiki is currently down.
> >
> > There should be a way to determine if the user has credentials before
> > trying to connect.
> >
> > -Kirk
> >
> >
> > On Thu, Oct 13, 2016 at 2:50 PM, Kevin Duling 
> wrote:
> >
> > > I'm working on GEODE-1959
> > >  and
> > > the use case goes like this:
> > >
> > > Create your gemfire.properites with a security manager.  For example:
> > >
> > > > security-manager=org.apache.geode.security.templates.
> > > SampleSecurityManage
> > >
> > > But do not provide a username and password within it.  Point to a
> > > security.json file that contains users and roles.  My startup line
> within
> > > gfsh goes like this:
> > >
> > > *gfsh>*start locator --name=loc --classpath=/Users/kduling/geode/run
> > > > --properties-file=./gemfire.properties
> > >
> > >
> > > Then start a server with security:
> > >
> > > *gfsh>*start server --name=secured --classpath=/Users/kduling/
> geode/run
> > > > --properties-file=./gemfire.properties --locators=127.0.0.1[10334]
> > >
> > >
> > > This produces a GemFireSecurityException because there are no login
> > > credentials set.  The entire stacktrace goes:
> > >
> > > objc[2117]: Class JavaLaunchHelper is implemented in both
> > > > /Library/Java/JavaVirtualMachines/jdk1.8.0_
> > 92.jdk/Contents/Home/jre/bin/
> > > java
> > > > and
> > > > /Library/Java/JavaVirtualMachines/jdk1.8.0_
> > 92.jdk/Contents/Home/jre/lib/
> > > libinstrument.dylib.
> > > > One of the two will be used. Which one is undefined.
> > > > Exception in thread "main"
> > > > org.apache.geode.security.GemFireSecurityException: Failed to find
> > > > credentials from [10.0.2.35(secured:2117):1025]
> > > > at
> > > > org.apache.geode.distributed.internal.membership.gms.
> > > membership.GMSJoinLeave.attemptToJoin(GMSJoinLeave.java:416)
> > > > at
> > > > org.apache.geode.distributed.internal.membership.gms.
> > > membership.GMSJoinLeave.join(GMSJoinLeave.java:314)
> > > > at
> > > > org.apache.geode.distributed.internal.membership.gms.mgr.
> > > GMSMembershipManager.join(GMSMembershipManager.java:662)
> > > > at
> > > > org.apache.geode.distributed.internal.membership.gms.mgr.
> > > GMSMembershipManager.joinDistributedSystem(
> > GMSMembershipManager.java:749)
> > > > at
> > > > org.apache.geode.distributed.internal.membership.gms.
> > > Services.start(Services.java:183)
> > > > at
> > > > org.apache.geode.distributed.internal.membership.gms.
> GMSMemberFactory.
> > > newMembershipManager(GMSMemberFactory.java:104)
> > > > at
> > > > org.apache.geode.distributed.internal.membership.MemberFactory.
> > > newMembershipManager(MemberFactory.java:92)
> > > > at
> > > > org.apache.geode.distributed.internal.DistributionManager.<
> > > init>(DistributionManager.java:1092)
> > > > at
> > > > org.apache.geode.distributed.internal.DistributionManager.<
> > > init>(DistributionManager.java:1144)
> > > > at
> > > > org.apache.geode.distributed.internal.DistributionManager.
> > > create(DistributionManager.java:521)
> > > > at
> > > > org.apache.geode.distributed.internal.InternalDistributedSystem.
> > > initialize(InternalDistributedSystem.java:657)
> > > > at
> > > > org.apache.geode.distributed.internal.InternalDistributedSystem.
> > > newInstance(InternalDistributedSystem.java:297)
> > > > at
> > > > org.apache.geode.distributed.DistributedSystem.connect(
> > > DistributedSystem.java:237)
> > > > at org.apache.geode.cache.CacheFactory.create(
> > CacheFactory.java:229)
> > > > at
> > > > org.apache.geode.distributed.internal.DefaultServerLauncherCacheProv
> > > ider.createCache(DefaultServerLauncherCacheProvider.java:55)
> > > > at
> > > > org.apache.geode.distributed.ServerLauncher.createCache(
> > > ServerLauncher.java:783)
> > > > at
> > > > org.apache.geode.distributed.ServerLauncher.start(
> > > ServerLauncher.java:703)
> > > > at
> > > > org.apache.geode.distributed.ServerLauncher.run(
> > ServerLauncher.java:633)
> > > > at
> > > > org.apache.geode.distributed.ServerLauncher.main(
> > > ServerLauncher.java:184)
> > >
> > >
> > > To resolve this, I was planning on catching the exception at
> > > DistributedSystem.connect() and then prompting for the
> username/password
> > > similar to what is done in ShellCommands.jmxConnect().
> > >
> > > 

Re: Deprecate DynamicRegionFactory

[Jinmei Liao]

+1 drop it if we don't want people to use it.

On Tue, Oct 11, 2016 at 8:15 PM, Kirk Lund  wrote:

> +1 drop it as a bug with no need for replacement (or in favor of using a
> function)
>
> On Tuesday, October 11, 2016, William Markito  wrote:
>
> > +1 -  We always discouraged people using dynamic regions
> >
> > On Tue, Oct 11, 2016 at 4:33 PM, Dan Smith  > > wrote:
> >
> > > I'd like to reopen the discussion about deprecating
> > > DynamicRegionFactory. This is some old crufty code I don't think we
> > > want anyone to use. I see we had a ticket to deprecated it
> > > (GEODE-388), but we chose not to do it because we don't have an
> > > alternative API to create regions from the client (GEODE-215).
> > >
> > > The thing is, there is a pretty easy workaround to use functions to
> > > create regions from the client. Personally I'd recommend that over
> > > DynamicRegionFactory. Can we just deprecate DynamicRegionFactory now
> > > and implement GEODE-215 later?
> > >
> > > -Dan
> > >
> >
> >
> >
> > --
> >
> > ~/William
> >
>



-- 
Cheers

Jinmei

Re: Review Request 52751: GEODE-1986: correctly set the flag indicating if cluster configuration service is running or not on a locator.

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52751/
---

(Updated Oct. 12, 2016, 2:21 a.m.)


Review request for geode, Jared Stewart, John Blum, Kevin Duling, and Kirk Lund.


Repository: geode


Description
---

GEODE-1986: correctly set the flag indicating if cluster configuration service 
is running or not on a locator.


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
 17303c1f984a90159f924a262d0ec843410cec16 
  
geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java 
d166397e3d5897347500e5447637fb4b99895388 
  
geode-core/src/test/java/org/apache/geode/security/ClusterConfigWithEmbededLocatorDUnitTest.java
 PRE-CREATION 

Diff: https://reviews.apache.org/r/52751/diff/


Testing
---

prechecking running


Thanks,

Jinmei Liao

Re: Review Request 52742: GEODE-1979: refactor tests using LocatorServerConfigurationRule

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52742/
---

(Updated Oct. 12, 2016, 1:11 a.m.)


Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.


Repository: geode


Description
---

GEODE-1979: refactor tests using LocatorServerConfigurationRule


Diffs
-

  
geode-core/src/test/java/org/apache/geode/security/ClusterConfigWithoutSecurityDUnitTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/security/SecurityClusterConfigDUnitTest.java
 ff7f0282ae0bd0bed74aca6e549c8b3162d76900 
  
geode-core/src/test/java/org/apache/geode/security/SecurityWithoutClusterConfigDUnitTest.java
 877f0571d6b6985d0cda43c35f0abae6cbc12a88 
  
geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java
 dae062acb980a93d00b5498e6ac99eda97918a39 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerConfigurationRule.java
 2fa262c732662becbba0eb48ce47e7d88b0fec5c 
  
geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java
 f837530e2bcd6026102d5a1267a48ac8b37578fe 

Diff: https://reviews.apache.org/r/52742/diff/


Testing (updated)
---

precheckin running


Thanks,

Jinmei Liao

Re: Review Request 52742: GEODE-1979: refactor tests using LocatorServerConfigurationRule

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52742/
---

(Updated Oct. 12, 2016, 1:10 a.m.)


Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.


Repository: geode


Description
---

GEODE-1979: refactor tests using LocatorServerConfigurationRule


Diffs (updated)
-

  
geode-core/src/test/java/org/apache/geode/security/ClusterConfigWithoutSecurityDUnitTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/security/SecurityClusterConfigDUnitTest.java
 ff7f0282ae0bd0bed74aca6e549c8b3162d76900 
  
geode-core/src/test/java/org/apache/geode/security/SecurityWithoutClusterConfigDUnitTest.java
 877f0571d6b6985d0cda43c35f0abae6cbc12a88 
  
geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java
 dae062acb980a93d00b5498e6ac99eda97918a39 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerConfigurationRule.java
 2fa262c732662becbba0eb48ce47e7d88b0fec5c 
  
geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java
 f837530e2bcd6026102d5a1267a48ac8b37578fe 

Diff: https://reviews.apache.org/r/52742/diff/


Testing
---

the affected tests


Thanks,

Jinmei Liao

Review Request 52751: GEODE-1986: correctly set the flag indicating if cluster configuration service is running or not on a locator.

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52751/
---

Review request for geode, Jared Stewart, John Blum, Kevin Duling, and Kirk Lund.


Repository: geode


Description
---

GEODE-1986: correctly set the flag indicating if cluster configuration service 
is running or not on a locator.


Diffs
-

  
geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
 17303c1f984a90159f924a262d0ec843410cec16 
  
geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java 
d166397e3d5897347500e5447637fb4b99895388 
  
geode-core/src/test/java/org/apache/geode/security/ClusterConfigWithEmbededLocatorDUnitTest.java
 PRE-CREATION 

Diff: https://reviews.apache.org/r/52751/diff/


Testing
---

prechecking running


Thanks,

Jinmei Liao

Review Request 52742: GEODE-1979: refactor tests using LocatorServerConfigurationRule

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52742/
---

Review request for geode.


Repository: geode


Description
---

GEODE-1979: refactor tests using LocatorServerConfigurationRule


Diffs
-

  
geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java
 dae062acb980a93d00b5498e6ac99eda97918a39 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerConfigurationRule.java
 2fa262c732662becbba0eb48ce47e7d88b0fec5c 
  
geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java
 f837530e2bcd6026102d5a1267a48ac8b37578fe 

Diff: https://reviews.apache.org/r/52742/diff/


Testing
---

the affected tests


Thanks,

Jinmei Liao

Re: Review Request 52724: GEODE-1570: improve rest security framework

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52724/
---

(Updated Oct. 11, 2016, 1:31 a.m.)


Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.


Repository: geode


Description
---

* use annotations for authorization
* consolidate the test classes


Diffs (updated)
-

  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java
 7d232ce6332175287c02af46b4b83309f41f66f1 
  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityEndpointsDUnitTest.java
 149a905af9d0ed0fb88b6a23d08d598546cf29a1 
  
geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
 a515de5b3a08eff3d78aed31c22b374cd51dbffe 
  
geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
 d645bbf2f37510378b39576f56239bc1aa5c9a07 
  geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java 
d6be5402fbee9d4a18c3e143384fed2e83acbda7 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/AbstractBaseController.java
 ee5d7146011af843c2d403bc67cd3fa5aa836547 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/BaseControllerAdvice.java
 54dd9e51f9a44183aea10378e243f96ba9d4b456 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/CommonCrudController.java
 232f0345e7db4503500ed03f1b0097ec830e6d6c 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/FunctionAccessController.java
 8cec1101b355e703d882bae8591c56c52f74d12e 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java
 c05845ad612d7ad43656af099c5c50dafb3c9bb8 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/QueryAccessController.java
 2df95aac231bf947cac7615069a28f65c50dd6fa 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/GeodeAuthentication.java
 c4226f6ab46b42ae3884365804df1d2010cc9de3 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/GeodeAuthenticationProvider.java
 c482047cb49d5d06f726d52e9157bd91c7b8ef72 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/GeodeAuthority.java
 fd216287e663db8e738683784da5461f5f7d8c9c 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/RestSecurityService.java
 PRE-CREATION 

Diff: https://reviews.apache.org/r/52724/diff/


Testing
---

precheckin running


Thanks,

Jinmei Liao

Review Request 52724: GEODE-1570: improve rest security framework

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52724/
---

Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.


Repository: geode


Description
---

* use annotations for authorization
* consolidate the test classes


Diffs
-

  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java
 7d232ce6332175287c02af46b4b83309f41f66f1 
  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityEndpointsDUnitTest.java
 149a905af9d0ed0fb88b6a23d08d598546cf29a1 
  
geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
 a515de5b3a08eff3d78aed31c22b374cd51dbffe 
  
geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
 d645bbf2f37510378b39576f56239bc1aa5c9a07 
  geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java 
d6be5402fbee9d4a18c3e143384fed2e83acbda7 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/AbstractBaseController.java
 ee5d7146011af843c2d403bc67cd3fa5aa836547 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/BaseControllerAdvice.java
 54dd9e51f9a44183aea10378e243f96ba9d4b456 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/CommonCrudController.java
 232f0345e7db4503500ed03f1b0097ec830e6d6c 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/FunctionAccessController.java
 8cec1101b355e703d882bae8591c56c52f74d12e 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java
 c05845ad612d7ad43656af099c5c50dafb3c9bb8 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/QueryAccessController.java
 2df95aac231bf947cac7615069a28f65c50dd6fa 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/GeodeAuthentication.java
 c4226f6ab46b42ae3884365804df1d2010cc9de3 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/GeodeAuthenticationProvider.java
 c482047cb49d5d06f726d52e9157bd91c7b8ef72 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/GeodeAuthority.java
 fd216287e663db8e738683784da5461f5f7d8c9c 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/RestSecurityService.java
 PRE-CREATION 

Diff: https://reviews.apache.org/r/52724/diff/


Testing
---

precheckin running


Thanks,

Jinmei Liao

Re: Review Request 52655: GEODE-1973: add more tests to cover GMSAuthenticator and SimpleSecurityManager

[Jinmei Liao]

> On Oct. 7, 2016, 11:21 p.m., Kirk Lund wrote:
> > geode-core/src/main/java/org/apache/geode/security/templates/SimpleSecurityManager.java,
> >  line 9
> > <https://reviews.apache.org/r/52655/diff/1/?file=1527748#file1527748line9>
> >
> > This class in the external packages of src/main. If we're not exposing 
> > this purposely for Users then it should move to an internal package. If 
> > it's exposed, then we need fancy javadocs and the Docs team will need to 
> > write about it too. 
> > 
> > I recommend moving to internal package.

It's meant to be serving the same purpose as SampleSecurityManager, but a 
simpler implementation without involving json files and such. This makes 
playing with security a lot easier. Our tests can use this instead of 
SampleSecurityManager as well for easier code flow. I will add fancy javadocs 
and let doc team know about it.


> On Oct. 7, 2016, 11:21 p.m., Kirk Lund wrote:
> > geode-cq/src/test/java/org/apache/geode/security/CQClientAuthDunitTest.java,
> >  line 55
> > <https://reviews.apache.org/r/52655/diff/1/?file=1527752#file1527752line55>
> >
> > I'd recommend changing this to INFO or just remove the line. FINE will 
> > create lots of output.

Sounds good.


- Jinmei


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52655/#review151900
---


On Oct. 7, 2016, 10:26 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52655/
> ---
> 
> (Updated Oct. 7, 2016, 10:26 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * Add a SimpleSecurityManager
> * Add more tests to cover GMSAuthenticator for cached and cacheless locator
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/security/templates/SimpleSecurityManager.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/security/PeerAuthenticatorDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/security/PeerAuthenticatorWithCachelessLocatorDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/security/templates/SimpleSecurityManagerTest.java
>  PRE-CREATION 
>   geode-cq/src/test/java/org/apache/geode/security/CQClientAuthDunitTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/52655/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Review Request 52655: GEODE-1973: add more tests to cover GMSAuthenticator and SimpleSecurityManager

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52655/
---

Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.


Repository: geode


Description
---

* Add a SimpleSecurityManager
* Add more tests to cover GMSAuthenticator for cached and cacheless locator


Diffs
-

  
geode-core/src/main/java/org/apache/geode/security/templates/SimpleSecurityManager.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/security/PeerAuthenticatorDUnitTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/security/PeerAuthenticatorWithCachelessLocatorDUnitTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/security/templates/SimpleSecurityManagerTest.java
 PRE-CREATION 
  geode-cq/src/test/java/org/apache/geode/security/CQClientAuthDunitTest.java 
PRE-CREATION 

Diff: https://reviews.apache.org/r/52655/diff/


Testing
---


Thanks,

Jinmei Liao

Re: Review Request 52650: GEODE-1973: having GMSAuthenticator work on a locator with no cache.

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52650/
---

(Updated Oct. 7, 2016, 10:09 p.m.)


Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.


Changes
---

Fix up review to mention GEODE-1973


Summary (updated)
-

GEODE-1973: having GMSAuthenticator work on a locator with no cache.


Repository: geode


Description (updated)
---

GEODE-1973: having GMSAuthenticator work on a locator with no cache.


Diffs
-

  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java
 8e4c15d4707ee6a6d48ab8be26a161e2a2660055 

Diff: https://reviews.apache.org/r/52650/diff/


Testing
---

hydra test passing


Thanks,

Jinmei Liao

Re: Review Request 52650: GEM-1032: having GMSAuthenticator work on a locator with no cache.

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52650/
---

(Updated Oct. 7, 2016, 10:07 p.m.)


Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.


Repository: geode


Description
---

GEM-1032: having GMSAuthenticator work on a locator with no cache.


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java
 8e4c15d4707ee6a6d48ab8be26a161e2a2660055 

Diff: https://reviews.apache.org/r/52650/diff/


Testing
---

hydra test passing


Thanks,

Jinmei Liao

Review Request 52650: GEM-1032: having GMSAuthenticator work on a locator with no cache.

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52650/
---

Review request for geode.


Repository: geode


Description
---

GEM-1032: having GMSAuthenticator work on a locator with no cache.


Diffs
-

  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java
 8e4c15d4707ee6a6d48ab8be26a161e2a2660055 

Diff: https://reviews.apache.org/r/52650/diff/


Testing
---

hydra test passing


Thanks,

Jinmei Liao

Re: Review Request 52621: GEM-1032: GMSAuthenticator needs to get security properties from system properties

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52621/
---

(Updated Oct. 6, 2016, 11:03 p.m.)


Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.


Repository: geode


Description
---

* adding more tests


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java
 8e4c15d4707ee6a6d48ab8be26a161e2a2660055 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticatorWithAuthenticatorTest.java
 c6cbf88c31362444f113785cdd217fdac8265461 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticatorWithSecurityManagerTest.java
 49475e0c0296287405d43dfb9d29dcd8e45305d4 
  
geode-core/src/test/java/org/apache/geode/security/PeerAuthenticatorDUnitTest.java
 PRE-CREATION 
  geode-cq/src/test/java/org/apache/geode/security/CQClientAuthDunitTest.java 
PRE-CREATION 

Diff: https://reviews.apache.org/r/52621/diff/


Testing
---

revert back to the old way of getting the security properties since this throws 
off some tests. Precheckin running


Thanks,

Jinmei Liao

Review Request 52621: GEM-1032: GMSAuthenticator needs to get security properties from system properties

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52621/
---

Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.


Repository: geode


Description
---

* adding more tests


Diffs
-

  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java
 8e4c15d4707ee6a6d48ab8be26a161e2a2660055 
  
geode-core/src/test/java/org/apache/geode/security/PeerAuthenticatorDUnitTest.java
 PRE-CREATION 
  geode-cq/src/test/java/org/apache/geode/security/CQClientAuthDunitTest.java 
PRE-CREATION 

Diff: https://reviews.apache.org/r/52621/diff/


Testing
---

revert back to the old way of getting the security properties since this throws 
off some tests. Precheckin running


Thanks,

Jinmei Liao

Re: Review Request 52571: GEODE-1570: upgrade spring libraries

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52571/
---

(Updated Oct. 6, 2016, 6:34 p.m.)


Review request for geode, Anthony Baker, Jared Stewart, Kevin Duling, Kirk 
Lund, and Dan Smith.


Repository: geode


Description
---

GEODE-1570: upgrade spring libraries and related libraries

* upgrade the spring related libraries
* rafactored the tests to get rid of redundant code
* update the NOTICE files


Diffs (updated)
-

  geode-assembly/src/main/dist/NOTICE 1924007f6c4822f59e0a830acf4e2b01f67170ef 
  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestInterfaceJUnitTest.java
 0d9351889b54b3ffdd6cba4e7ade4ae5e5a17093 
  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java
 a9d90ed5489a9ab6b936674369b5e5a870651738 
  geode-assembly/src/test/resources/expected_jars.txt 
939464a92a3f1846b8fb9b9d1faa75dad5133289 
  geode-core/build.gradle 3cbdfbea41a7f34b237394e92a463919d0109f47 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/shell/GfshInitFileJUnitTest.java
 9e7b7bf17920a9dc42905bab4ccb4590ca20223a 
  
geode-core/src/test/java/org/apache/geode/security/AbstractSecureServerDUnitTest.java
 435b426627deea6c67ecd79bc014ec97774e2ebf 
  
geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthentication.java
 425f5a5aa76c8219678164ebbb012d1361519bc7 
  
geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthenticationProvider.java
 f4575ccf5c894e2cbaaebc51ffc4d7f56fce68c4 
  
geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/MemberGatewayHubService.java
 51abb3f39902938149fb67a416f571ba31b75d77 
  geode-pulse/src/main/webapp/Login.html 
f22490f4df098c0b12dadeefcb1855a51efc6281 
  geode-pulse/src/main/webapp/META-INF/NOTICE 
40f9fc875d6bc53d073061495738f636ad60433c 
  geode-pulse/src/main/webapp/WEB-INF/mvc-dispatcher-servlet.xml 
cb7181d3279e2c2dfc86ddc201fa6dc002eb55fe 
  geode-pulse/src/main/webapp/WEB-INF/spring-security.xml 
924dd50f579a6e4e306b94f4a7fd88e87f978add 
  
geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAuthTest.java 
a77e0cacdcde69f47fbd13c588601072c432ee0d 
  
geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAutomatedTest.java
 a4f14f89ea739e7390565d652fc5affa6e1326c6 
  geode-web-api/build.gradle 3ea652b65456168689250f35be553c2eeda193d9 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/JSONUtils.java
 ccf3b9d9f0d0aebadbf628ef6bdcef1084e81b5d 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/JsonWriter.java
 1fcffbd4eee26fee890f158794727647145a1914 
  geode-web-api/src/main/webapp/META-INF/NOTICE 
2f4da6e85a2daa75e136bbc80e62768193f65db9 
  geode-web-api/src/main/webapp/WEB-INF/geode-servlet.xml 
c75d975c4313f953cce5ac7f5a8cd8a228718a8c 
  geode-web/src/main/webapp/META-INF/NOTICE 
dd46891056a99365494ac212fec8ef8b3dd0a201 
  geode-web/src/main/webapp/WEB-INF/geode-mgmt-servlet.xml 
3b1d1658ea2414190852cc1268fd9f1beccf4d20 
  gradle/dependency-resolution.gradle 91d1755848ba9ce23fab3190555c2906bcffd97b 
  gradle/dependency-versions.properties 
65fd2ee1d46156781ee70165aac578c5ca14490a 

Diff: https://reviews.apache.org/r/52571/diff/


Testing
---

precheckin running.


Thanks,

Jinmei Liao

Re: Review Request 52571: GEODE-1570: upgrade spring libraries

[Jinmei Liao]

/META-INF/NOTICE 
dd46891056a99365494ac212fec8ef8b3dd0a201 
  geode-web/src/main/webapp/WEB-INF/geode-mgmt-servlet.xml 
3b1d1658ea2414190852cc1268fd9f1beccf4d20 
  gradle.properties 1882b7b3fd55a17db962aa6d08179e572c72fffc 
  gradle/dependency-resolution.gradle 91d1755848ba9ce23fab3190555c2906bcffd97b 
  gradle/dependency-versions.properties 
0abe690d2484258edded988abbfac24e560fbff8 

Diff: https://reviews.apache.org/r/52571/diff/


Testing
---

precheckin running.


Thanks,

Jinmei Liao

Assert usage in geode-core

[Jinmei Liao]

Is there a initiative to get spring-core dependency out of geode-core? The
only places we are using spring-core classes in geode-core are those Assert
statements like: Assert.isNull, Assert.notNull etc. Should we try to get
rid of those?

-- 
Cheers

Jinmei

Re: Review Request 52571: GEODE-1570: upgrade spring libraries

[Jinmei Liao]

You must be looking at some wrong patch, or the patch did not apply
correctly.  The tests are passing:
http://test-results-jinmei-1570-2.s3-website-us-west-2.amazonaws.com/test
http://test-results-jinmei-1570-2.s3-website-us-west-2.
amazonaws.com/flakyTest/

On Wed, Oct 5, 2016 at 3:33 PM, Jared Stewart <jstew...@pivotal.io> wrote:

> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52571/
>
> I also have a few unit tests failing from this patch:
>
> org.apache.geode.management.internal.cli.shell.GfshInitFileJUnitTest > 
> testInitFile_TwoBadCommands FAILED
> java.lang.AssertionError: Log records written expected:<4> but was:<3>
> at org.junit.Assert.fail(Assert.java:88)
> at org.junit.Assert.failNotEquals(Assert.java:834)
> at org.junit.Assert.assertEquals(Assert.java:645)
> at 
> org.apache.geode.management.internal.cli.shell.GfshInitFileJUnitTest.testInitFile_TwoBadCommands(GfshInitFileJUnitTest.java:402)
>
> org.apache.geode.management.internal.cli.shell.GfshInitFileJUnitTest > 
> testInitFile_OneBadCommand FAILED
> java.lang.AssertionError: Log records written expected:<4> but was:<3>
> at org.junit.Assert.fail(Assert.java:88)
> at org.junit.Assert.failNotEquals(Assert.java:834)
> at org.junit.Assert.assertEquals(Assert.java:645)
> at 
> org.apache.geode.management.internal.cli.shell.GfshInitFileJUnitTest.testInitFile_OneBadCommand(GfshInitFileJUnitTest.java:371)
>
> org.apache.geode.management.internal.cli.shell.GfshInitFileJUnitTest > 
> testInitFile_BadAndGoodCommands FAILED
> java.lang.AssertionError: Log records written expected:<4> but was:<3>
> at org.junit.Assert.fail(Assert.java:88)
> at org.junit.Assert.failNotEquals(Assert.java:834)
> at org.junit.Assert.assertEquals(Assert.java:645)
> at 
> org.apache.geode.management.internal.cli.shell.GfshInitFileJUnitTest.testInitFile_BadAndGoodCommands(GfshInitFileJUnitTest.java:434)
>
>
> - Jared Stewart
>
> On October 5th, 2016, 9:06 p.m. UTC, Jinmei Liao wrote:
> Review request for geode, Anthony Baker, Jared Stewart, Kevin Duling, Kirk
> Lund, and Dan Smith.
> By Jinmei Liao.
>
> *Updated Oct. 5, 2016, 9:06 p.m.*
> *Repository: * geode
> Description
>
> GEODE-1570: upgrade spring libraries and related libraries
>
>- upgrade the spring related libraries
>- rafactored the tests to get rid of redundant code
>
> Testing
>
> precheckin running.
>
> Diffs
>
>- 
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestInterfaceJUnitTest.java
>(0d9351889b54b3ffdd6cba4e7ade4ae5e5a17093)
>- 
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java
>(a9d90ed5489a9ab6b936674369b5e5a870651738)
>- geode-assembly/src/test/resources/expected_jars.txt (
>939464a92a3f1846b8fb9b9d1faa75dad5133289)
>- geode-core/build.gradle (3cbdfbea41a7f34b237394e92a463919d0109f47)
>- geode-core/src/test/java/org/apache/geode/management/
>internal/cli/shell/GfshInitFileJUnitTest.java (
>9e7b7bf17920a9dc42905bab4ccb4590ca20223a)
>- geode-core/src/test/java/org/apache/geode/security/
>AbstractSecureServerDUnitTest.java (435b426627deea6c67ecd79bc014ec
>97774e2ebf)
>- geode-pulse/src/main/java/org/apache/geode/tools/pulse/
>internal/security/GemFireAuthentication.java (
>425f5a5aa76c8219678164ebbb012d1361519bc7)
>- geode-pulse/src/main/java/org/apache/geode/tools/pulse/
>internal/security/GemFireAuthenticationProvider.java (
>f4575ccf5c894e2cbaaebc51ffc4d7f56fce68c4)
>- geode-pulse/src/main/java/org/apache/geode/tools/pulse/
>internal/service/MemberGatewayHubService.java (
>51abb3f39902938149fb67a416f571ba31b75d77)
>- geode-pulse/src/main/webapp/Login.html (
>f22490f4df098c0b12dadeefcb1855a51efc6281)
>- geode-pulse/src/main/webapp/WEB-INF/mvc-dispatcher-servlet.xml (
>cb7181d3279e2c2dfc86ddc201fa6dc002eb55fe)
>- geode-pulse/src/main/webapp/WEB-INF/spring-security.xml (
>924dd50f579a6e4e306b94f4a7fd88e87f978add)
>- 
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAuthTest.java
>(a77e0cacdcde69f47fbd13c588601072c432ee0d)
>- 
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAutomatedTest.java
>(a4f14f89ea739e7390565d652fc5affa6e1326c6)
>- geode-web-api/src/main/java/org/apache/geode/rest/
>internal/web/util/JSONUtils.java (ccf3b9d9f0d0aebadbf628ef6bdcef
>1084e81b5d)
>- geode-web-api/src/main/java/org/apache/geode/rest/
>internal/web/util/JsonWriter.java (1fcffbd4

Re: Review Request 52571: GEODE-1570: upgrade spring libraries

[Jinmei Liao]

> On Oct. 5, 2016, 9:18 p.m., Jared Stewart wrote:
> > Aren't the (OutputStream) casts in JSONUtils.java redundant since 
> > outputStream here is already an instance of HeapDataOutputStream which 
> > extends OutputStream?
> > 
> > Also, I believe our schemaLocations in *-servlet.xml ought to refer to 
> > versioned URLs so that we do not break when a new Spring version is 
> > released before we have updated to that version.
> 
> Kevin Duling wrote:
> When I was trying to do this ticket, I ran in to the necessity of having 
> to cast because the compiler objected, saying there was no matching method 
> that would accept it -- even though it clearly appeared there was.  By 
> casting, it resolved it.  I'm sure Jinmei ran in to the same issue when she 
> worked on the upgrade.

If I remeber correctly, if I don't cast, it complains about some method 
ambiguity.

And for schemeLocations, It is recommended to use the "versionless" XSDs, 
because they're mapped to the current version of the framework you're using in 
your application. 
http://stackoverflow.com/questions/20894695/spring-configuration-xml-schema-with-or-without-version


- Jinmei


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52571/#review151560
-------


On Oct. 5, 2016, 9:06 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52571/
> ---
> 
> (Updated Oct. 5, 2016, 9:06 p.m.)
> 
> 
> Review request for geode, Anthony Baker, Jared Stewart, Kevin Duling, Kirk 
> Lund, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1570: upgrade spring libraries and related libraries
> 
> * upgrade the spring related libraries
> * rafactored the tests to get rid of redundant code
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestInterfaceJUnitTest.java
>  0d9351889b54b3ffdd6cba4e7ade4ae5e5a17093 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java
>  a9d90ed5489a9ab6b936674369b5e5a870651738 
>   geode-assembly/src/test/resources/expected_jars.txt 
> 939464a92a3f1846b8fb9b9d1faa75dad5133289 
>   geode-core/build.gradle 3cbdfbea41a7f34b237394e92a463919d0109f47 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/shell/GfshInitFileJUnitTest.java
>  9e7b7bf17920a9dc42905bab4ccb4590ca20223a 
>   
> geode-core/src/test/java/org/apache/geode/security/AbstractSecureServerDUnitTest.java
>  435b426627deea6c67ecd79bc014ec97774e2ebf 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthentication.java
>  425f5a5aa76c8219678164ebbb012d1361519bc7 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthenticationProvider.java
>  f4575ccf5c894e2cbaaebc51ffc4d7f56fce68c4 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/MemberGatewayHubService.java
>  51abb3f39902938149fb67a416f571ba31b75d77 
>   geode-pulse/src/main/webapp/Login.html 
> f22490f4df098c0b12dadeefcb1855a51efc6281 
>   geode-pulse/src/main/webapp/WEB-INF/mvc-dispatcher-servlet.xml 
> cb7181d3279e2c2dfc86ddc201fa6dc002eb55fe 
>   geode-pulse/src/main/webapp/WEB-INF/spring-security.xml 
> 924dd50f579a6e4e306b94f4a7fd88e87f978add 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAuthTest.java
>  a77e0cacdcde69f47fbd13c588601072c432ee0d 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAutomatedTest.java
>  a4f14f89ea739e7390565d652fc5affa6e1326c6 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/JSONUtils.java
>  ccf3b9d9f0d0aebadbf628ef6bdcef1084e81b5d 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/JsonWriter.java
>  1fcffbd4eee26fee890f158794727647145a1914 
>   geode-web-api/src/main/webapp/WEB-INF/geode-servlet.xml 
> c75d975c4313f953cce5ac7f5a8cd8a228718a8c 
>   geode-web/src/main/webapp/WEB-INF/geode-mgmt-servlet.xml 
> 3b1d1658ea2414190852cc1268fd9f1beccf4d20 
>   gradle/dependency-resolution.gradle 
> 91d1755848ba9ce23fab3190555c2906bcffd97b 
>   gradle/dependency-versions.properties 
> 65fd2ee1d46156781ee70165aac578c5ca14490a 
> 
> Diff: https://reviews.apache.org/r/52571/diff/
> 
> 
> Testing
> ---
> 
> precheckin running.
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 52571: GEODE-1570: upgrade spring libraries

[Jinmei Liao]

> On Oct. 5, 2016, 9:47 p.m., Jared Stewart wrote:
> > I also get compilation errors in PulseAppListener and 
> > ExceptionHandlingAdvice when I apply this patch, even after refreshing 
> > gradle.

Hnnn, do a clean and refresh. I am not getting the errors.


- Jinmei


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52571/#review151569
---


On Oct. 5, 2016, 9:06 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52571/
> ---
> 
> (Updated Oct. 5, 2016, 9:06 p.m.)
> 
> 
> Review request for geode, Anthony Baker, Jared Stewart, Kevin Duling, Kirk 
> Lund, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1570: upgrade spring libraries and related libraries
> 
> * upgrade the spring related libraries
> * rafactored the tests to get rid of redundant code
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestInterfaceJUnitTest.java
>  0d9351889b54b3ffdd6cba4e7ade4ae5e5a17093 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java
>  a9d90ed5489a9ab6b936674369b5e5a870651738 
>   geode-assembly/src/test/resources/expected_jars.txt 
> 939464a92a3f1846b8fb9b9d1faa75dad5133289 
>   geode-core/build.gradle 3cbdfbea41a7f34b237394e92a463919d0109f47 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/shell/GfshInitFileJUnitTest.java
>  9e7b7bf17920a9dc42905bab4ccb4590ca20223a 
>   
> geode-core/src/test/java/org/apache/geode/security/AbstractSecureServerDUnitTest.java
>  435b426627deea6c67ecd79bc014ec97774e2ebf 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthentication.java
>  425f5a5aa76c8219678164ebbb012d1361519bc7 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthenticationProvider.java
>  f4575ccf5c894e2cbaaebc51ffc4d7f56fce68c4 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/MemberGatewayHubService.java
>  51abb3f39902938149fb67a416f571ba31b75d77 
>   geode-pulse/src/main/webapp/Login.html 
> f22490f4df098c0b12dadeefcb1855a51efc6281 
>   geode-pulse/src/main/webapp/WEB-INF/mvc-dispatcher-servlet.xml 
> cb7181d3279e2c2dfc86ddc201fa6dc002eb55fe 
>   geode-pulse/src/main/webapp/WEB-INF/spring-security.xml 
> 924dd50f579a6e4e306b94f4a7fd88e87f978add 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAuthTest.java
>  a77e0cacdcde69f47fbd13c588601072c432ee0d 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAutomatedTest.java
>  a4f14f89ea739e7390565d652fc5affa6e1326c6 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/JSONUtils.java
>  ccf3b9d9f0d0aebadbf628ef6bdcef1084e81b5d 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/JsonWriter.java
>  1fcffbd4eee26fee890f158794727647145a1914 
>   geode-web-api/src/main/webapp/WEB-INF/geode-servlet.xml 
> c75d975c4313f953cce5ac7f5a8cd8a228718a8c 
>   geode-web/src/main/webapp/WEB-INF/geode-mgmt-servlet.xml 
> 3b1d1658ea2414190852cc1268fd9f1beccf4d20 
>   gradle/dependency-resolution.gradle 
> 91d1755848ba9ce23fab3190555c2906bcffd97b 
>   gradle/dependency-versions.properties 
> 65fd2ee1d46156781ee70165aac578c5ca14490a 
> 
> Diff: https://reviews.apache.org/r/52571/diff/
> 
> 
> Testing
> ---
> 
> precheckin running.
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 52571: GEODE-1570: upgrade spring libraries

[Jinmei Liao]

> On Oct. 5, 2016, 9:24 p.m., Anthony Baker wrote:
> > gradle/dependency-resolution.gradle, line 18
> > <https://reviews.apache.org/r/52571/diff/1/?file=1523390#file1523390line18>
> >
> > What's different that allows us to remove this code?

by upgrading the libraries, we don't need these force statement anymore. It 
will just pull in the needed versions.


- Jinmei


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52571/#review151559
---


On Oct. 5, 2016, 9:06 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52571/
> ---
> 
> (Updated Oct. 5, 2016, 9:06 p.m.)
> 
> 
> Review request for geode, Anthony Baker, Jared Stewart, Kevin Duling, Kirk 
> Lund, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1570: upgrade spring libraries and related libraries
> 
> * upgrade the spring related libraries
> * rafactored the tests to get rid of redundant code
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestInterfaceJUnitTest.java
>  0d9351889b54b3ffdd6cba4e7ade4ae5e5a17093 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java
>  a9d90ed5489a9ab6b936674369b5e5a870651738 
>   geode-assembly/src/test/resources/expected_jars.txt 
> 939464a92a3f1846b8fb9b9d1faa75dad5133289 
>   geode-core/build.gradle 3cbdfbea41a7f34b237394e92a463919d0109f47 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/shell/GfshInitFileJUnitTest.java
>  9e7b7bf17920a9dc42905bab4ccb4590ca20223a 
>   
> geode-core/src/test/java/org/apache/geode/security/AbstractSecureServerDUnitTest.java
>  435b426627deea6c67ecd79bc014ec97774e2ebf 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthentication.java
>  425f5a5aa76c8219678164ebbb012d1361519bc7 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthenticationProvider.java
>  f4575ccf5c894e2cbaaebc51ffc4d7f56fce68c4 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/MemberGatewayHubService.java
>  51abb3f39902938149fb67a416f571ba31b75d77 
>   geode-pulse/src/main/webapp/Login.html 
> f22490f4df098c0b12dadeefcb1855a51efc6281 
>   geode-pulse/src/main/webapp/WEB-INF/mvc-dispatcher-servlet.xml 
> cb7181d3279e2c2dfc86ddc201fa6dc002eb55fe 
>   geode-pulse/src/main/webapp/WEB-INF/spring-security.xml 
> 924dd50f579a6e4e306b94f4a7fd88e87f978add 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAuthTest.java
>  a77e0cacdcde69f47fbd13c588601072c432ee0d 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAutomatedTest.java
>  a4f14f89ea739e7390565d652fc5affa6e1326c6 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/JSONUtils.java
>  ccf3b9d9f0d0aebadbf628ef6bdcef1084e81b5d 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/JsonWriter.java
>  1fcffbd4eee26fee890f158794727647145a1914 
>   geode-web-api/src/main/webapp/WEB-INF/geode-servlet.xml 
> c75d975c4313f953cce5ac7f5a8cd8a228718a8c 
>   geode-web/src/main/webapp/WEB-INF/geode-mgmt-servlet.xml 
> 3b1d1658ea2414190852cc1268fd9f1beccf4d20 
>   gradle/dependency-resolution.gradle 
> 91d1755848ba9ce23fab3190555c2906bcffd97b 
>   gradle/dependency-versions.properties 
> 65fd2ee1d46156781ee70165aac578c5ca14490a 
> 
> Diff: https://reviews.apache.org/r/52571/diff/
> 
> 
> Testing
> ---
> 
> precheckin running.
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 52571: GEODE-1570: upgrade spring libraries

[Jinmei Liao]

> On Oct. 5, 2016, 9:24 p.m., Anthony Baker wrote:
> > geode-assembly/src/test/resources/expected_jars.txt, line 20
> > <https://reviews.apache.org/r/52571/diff/1/?file=1523374#file1523374line20>
> >
> > Does this jar need to be on the geode/gfsh-dependencies.jar classpath?  
> > If so, add to the list in geode-assembly/build.gradle.

jackson-module-paranamer is pulled in by the latest version of 
jackson-module-scala_2.10. Looks like geode-web-api is using scala_2.10, but 
it's not used by gfsh.


- Jinmei


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52571/#review151559
-------


On Oct. 5, 2016, 9:06 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52571/
> ---
> 
> (Updated Oct. 5, 2016, 9:06 p.m.)
> 
> 
> Review request for geode, Anthony Baker, Jared Stewart, Kevin Duling, Kirk 
> Lund, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1570: upgrade spring libraries and related libraries
> 
> * upgrade the spring related libraries
> * rafactored the tests to get rid of redundant code
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestInterfaceJUnitTest.java
>  0d9351889b54b3ffdd6cba4e7ade4ae5e5a17093 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java
>  a9d90ed5489a9ab6b936674369b5e5a870651738 
>   geode-assembly/src/test/resources/expected_jars.txt 
> 939464a92a3f1846b8fb9b9d1faa75dad5133289 
>   geode-core/build.gradle 3cbdfbea41a7f34b237394e92a463919d0109f47 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/shell/GfshInitFileJUnitTest.java
>  9e7b7bf17920a9dc42905bab4ccb4590ca20223a 
>   
> geode-core/src/test/java/org/apache/geode/security/AbstractSecureServerDUnitTest.java
>  435b426627deea6c67ecd79bc014ec97774e2ebf 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthentication.java
>  425f5a5aa76c8219678164ebbb012d1361519bc7 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthenticationProvider.java
>  f4575ccf5c894e2cbaaebc51ffc4d7f56fce68c4 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/MemberGatewayHubService.java
>  51abb3f39902938149fb67a416f571ba31b75d77 
>   geode-pulse/src/main/webapp/Login.html 
> f22490f4df098c0b12dadeefcb1855a51efc6281 
>   geode-pulse/src/main/webapp/WEB-INF/mvc-dispatcher-servlet.xml 
> cb7181d3279e2c2dfc86ddc201fa6dc002eb55fe 
>   geode-pulse/src/main/webapp/WEB-INF/spring-security.xml 
> 924dd50f579a6e4e306b94f4a7fd88e87f978add 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAuthTest.java
>  a77e0cacdcde69f47fbd13c588601072c432ee0d 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAutomatedTest.java
>  a4f14f89ea739e7390565d652fc5affa6e1326c6 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/JSONUtils.java
>  ccf3b9d9f0d0aebadbf628ef6bdcef1084e81b5d 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/JsonWriter.java
>  1fcffbd4eee26fee890f158794727647145a1914 
>   geode-web-api/src/main/webapp/WEB-INF/geode-servlet.xml 
> c75d975c4313f953cce5ac7f5a8cd8a228718a8c 
>   geode-web/src/main/webapp/WEB-INF/geode-mgmt-servlet.xml 
> 3b1d1658ea2414190852cc1268fd9f1beccf4d20 
>   gradle/dependency-resolution.gradle 
> 91d1755848ba9ce23fab3190555c2906bcffd97b 
>   gradle/dependency-versions.properties 
> 65fd2ee1d46156781ee70165aac578c5ca14490a 
> 
> Diff: https://reviews.apache.org/r/52571/diff/
> 
> 
> Testing
> ---
> 
> precheckin running.
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 52571: GEODE-1570: upgrade spring libraries

[Jinmei Liao]

> On Oct. 5, 2016, 9:24 p.m., Anthony Baker wrote:
> > geode-core/build.gradle, line 90
> > <https://reviews.apache.org/r/52571/diff/1/?file=1523375#file1523375line90>
> >
> > We just eliminated spring-core from the classpath of geode-core...what 
> > code needs this jar?

spring-core is still pulled in geode-core by sping-shell. It's still used by 
GfshMethodTarget.


- Jinmei


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52571/#review151559
-------


On Oct. 5, 2016, 9:06 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52571/
> ---
> 
> (Updated Oct. 5, 2016, 9:06 p.m.)
> 
> 
> Review request for geode, Anthony Baker, Jared Stewart, Kevin Duling, Kirk 
> Lund, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1570: upgrade spring libraries and related libraries
> 
> * upgrade the spring related libraries
> * rafactored the tests to get rid of redundant code
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestInterfaceJUnitTest.java
>  0d9351889b54b3ffdd6cba4e7ade4ae5e5a17093 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java
>  a9d90ed5489a9ab6b936674369b5e5a870651738 
>   geode-assembly/src/test/resources/expected_jars.txt 
> 939464a92a3f1846b8fb9b9d1faa75dad5133289 
>   geode-core/build.gradle 3cbdfbea41a7f34b237394e92a463919d0109f47 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/shell/GfshInitFileJUnitTest.java
>  9e7b7bf17920a9dc42905bab4ccb4590ca20223a 
>   
> geode-core/src/test/java/org/apache/geode/security/AbstractSecureServerDUnitTest.java
>  435b426627deea6c67ecd79bc014ec97774e2ebf 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthentication.java
>  425f5a5aa76c8219678164ebbb012d1361519bc7 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthenticationProvider.java
>  f4575ccf5c894e2cbaaebc51ffc4d7f56fce68c4 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/MemberGatewayHubService.java
>  51abb3f39902938149fb67a416f571ba31b75d77 
>   geode-pulse/src/main/webapp/Login.html 
> f22490f4df098c0b12dadeefcb1855a51efc6281 
>   geode-pulse/src/main/webapp/WEB-INF/mvc-dispatcher-servlet.xml 
> cb7181d3279e2c2dfc86ddc201fa6dc002eb55fe 
>   geode-pulse/src/main/webapp/WEB-INF/spring-security.xml 
> 924dd50f579a6e4e306b94f4a7fd88e87f978add 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAuthTest.java
>  a77e0cacdcde69f47fbd13c588601072c432ee0d 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAutomatedTest.java
>  a4f14f89ea739e7390565d652fc5affa6e1326c6 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/JSONUtils.java
>  ccf3b9d9f0d0aebadbf628ef6bdcef1084e81b5d 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/JsonWriter.java
>  1fcffbd4eee26fee890f158794727647145a1914 
>   geode-web-api/src/main/webapp/WEB-INF/geode-servlet.xml 
> c75d975c4313f953cce5ac7f5a8cd8a228718a8c 
>   geode-web/src/main/webapp/WEB-INF/geode-mgmt-servlet.xml 
> 3b1d1658ea2414190852cc1268fd9f1beccf4d20 
>   gradle/dependency-resolution.gradle 
> 91d1755848ba9ce23fab3190555c2906bcffd97b 
>   gradle/dependency-versions.properties 
> 65fd2ee1d46156781ee70165aac578c5ca14490a 
> 
> Diff: https://reviews.apache.org/r/52571/diff/
> 
> 
> Testing
> ---
> 
> precheckin running.
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Review Request 52571: GEODE-1570: upgrade spring libraries

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52571/
---

Review request for geode, Anthony Baker, Jared Stewart, Kevin Duling, Kirk 
Lund, and Dan Smith.


Repository: geode


Description
---

GEODE-1570: upgrade spring libraries and related libraries

* upgrade the spring related libraries
* rafactored the tests to get rid of redundant code


Diffs
-

  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestInterfaceJUnitTest.java
 0d9351889b54b3ffdd6cba4e7ade4ae5e5a17093 
  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java
 a9d90ed5489a9ab6b936674369b5e5a870651738 
  geode-assembly/src/test/resources/expected_jars.txt 
939464a92a3f1846b8fb9b9d1faa75dad5133289 
  geode-core/build.gradle 3cbdfbea41a7f34b237394e92a463919d0109f47 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/shell/GfshInitFileJUnitTest.java
 9e7b7bf17920a9dc42905bab4ccb4590ca20223a 
  
geode-core/src/test/java/org/apache/geode/security/AbstractSecureServerDUnitTest.java
 435b426627deea6c67ecd79bc014ec97774e2ebf 
  
geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthentication.java
 425f5a5aa76c8219678164ebbb012d1361519bc7 
  
geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthenticationProvider.java
 f4575ccf5c894e2cbaaebc51ffc4d7f56fce68c4 
  
geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/MemberGatewayHubService.java
 51abb3f39902938149fb67a416f571ba31b75d77 
  geode-pulse/src/main/webapp/Login.html 
f22490f4df098c0b12dadeefcb1855a51efc6281 
  geode-pulse/src/main/webapp/WEB-INF/mvc-dispatcher-servlet.xml 
cb7181d3279e2c2dfc86ddc201fa6dc002eb55fe 
  geode-pulse/src/main/webapp/WEB-INF/spring-security.xml 
924dd50f579a6e4e306b94f4a7fd88e87f978add 
  
geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAuthTest.java 
a77e0cacdcde69f47fbd13c588601072c432ee0d 
  
geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAutomatedTest.java
 a4f14f89ea739e7390565d652fc5affa6e1326c6 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/JSONUtils.java
 ccf3b9d9f0d0aebadbf628ef6bdcef1084e81b5d 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/JsonWriter.java
 1fcffbd4eee26fee890f158794727647145a1914 
  geode-web-api/src/main/webapp/WEB-INF/geode-servlet.xml 
c75d975c4313f953cce5ac7f5a8cd8a228718a8c 
  geode-web/src/main/webapp/WEB-INF/geode-mgmt-servlet.xml 
3b1d1658ea2414190852cc1268fd9f1beccf4d20 
  gradle/dependency-resolution.gradle 91d1755848ba9ce23fab3190555c2906bcffd97b 
  gradle/dependency-versions.properties 
65fd2ee1d46156781ee70165aac578c5ca14490a 

Diff: https://reviews.apache.org/r/52571/diff/


Testing
---

precheckin running.


Thanks,

Jinmei Liao

Re: Review Request 52525: GEODE-1902: add ACCEPT and DENY tests for GEODE_VERBOSE and GEMFIRE_VERBOSE

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52525/#review151380
---


Ship it!




Ship It!

- Jinmei Liao


On Oct. 4, 2016, 8:23 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52525/
> ---
> 
> (Updated Oct. 4, 2016, 8:23 p.m.)
> 
> 
> Review request for geode and Kevin Duling.
> 
> 
> Bugs: GEODE-1902
> https://issues.apache.org/jira/browse/GEODE-1902
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1902: add ACCEPT and DENY tests for GEODE_VERBOSE and GEMFIRE_VERBOSE
> 
> log4j2-custom.xml isn't really sufficient for testing GEODE_VERBOSE and 
> GEMFIRE_VERBOSE. This changeset adds in new 4 log4j2 xml files that enable 
> and disable each of the VERBOSE markers.
> 
> * add two log4j2 xml files for GEMFIRE_VERBOSE: 
> log4j2-gemfire_verbose-accept.xml, log4j2-gemfire_verbose-deny.xml
> * add two log4j2 xml files for GEODE_VERBOSE: 
> log4j2-geode_verbose-accept.xml, log4j2-geode_verbose-deny.xml
> * rename test GeodeVerboseLogMarkerIntegrationTest
> * add test methods to verify every combination of behavior for accept and 
> deny xml files
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/org/apache/geode/internal/logging/log4j/Configuration.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/logging/log4j/GeodeVerboseLogMarkerIntegrationTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/logging/log4j/LogMarkerJUnitTest.java
>  ec9d257892b64bb6ad2fc58755a448c70ddf538d 
>   
> geode-core/src/test/resources/org/apache/geode/internal/logging/log4j/marker/log4j2-gemfire_verbose-accept.xml
>  PRE-CREATION 
>   
> geode-core/src/test/resources/org/apache/geode/internal/logging/log4j/marker/log4j2-gemfire_verbose-deny.xml
>  PRE-CREATION 
>   
> geode-core/src/test/resources/org/apache/geode/internal/logging/log4j/marker/log4j2-geode_verbose-accept.xml
>  PRE-CREATION 
>   
> geode-core/src/test/resources/org/apache/geode/internal/logging/log4j/marker/log4j2-geode_verbose-deny.xml
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/52525/diff/
> 
> 
> Testing
> ---
> 
> GeodeVerboseLogMarkerIntegrationTest
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Re: Review Request 52488: GEODE-420: fix Pulse test when not using any SSLConfig

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52488/
---

(Updated Oct. 3, 2016, 11:20 p.m.)


Review request for geode, Jared Stewart, Kevin Duling, Kirk Lund, and Udo 
Kohlmeyer.


Changes
---

use default SSLConfig instead of passing in null

test passed


Repository: geode


Description
---

GEODE-420: fix Pulse test when not using any SSLConfig


Diffs
-

  
geode-core/src/main/java/org/apache/geode/management/internal/JettyHelper.java 
089dbacb056da84d6d6ceb36dad6f09b9ce73890 
  
geode-pulse/src/test/java/org/apache/geode/tools/pulse/testbed/driver/PulseUITest.java
 a2365a2dfa3e221fa50664c3f8d96fae2bd93c86 
  
geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAbstractTest.java
 5024250f8ff3867e0f527d7ef5fbbd501e2d64ee 

Diff: https://reviews.apache.org/r/52488/diff/


Testing (updated)
---

precheckin passed


Thanks,

Jinmei Liao

Re: Review Request 52488: GEODE-420: fix Pulse test when not using any SSLConfig

[Jinmei Liao]

> On Oct. 3, 2016, 6:54 p.m., Udo Kohlmeyer wrote:
> > geode-core/src/main/java/org/apache/geode/management/internal/JettyHelper.java,
> >  line 200
> > <https://reviews.apache.org/r/52488/diff/1/?file=1518865#file1518865line200>
> >
> > I don't think you should pass null. The SSLConfigurationFactory will 
> > always provide you with a SSLConfig object. This object might be a 
> > ssl-enabled=false, but that is ok. I think this should should be replaced 
> > with:
> > 
> > SSLConfigurationFactory.setDistributionConfig(new DistributionConfig);
> > final Server jetty = JettyHelper.initJetty(null, 8090, 
> > SSLConfigurationFactory.getSSLConfigForComponent(SecurableCommunicationChannel.WEB));

DistributionConfig is an interface, and if I use DistributionConfigImpl(), I 
get compiling error as well. I understand your concern of "checking null", but 
by forcing callers to pass in a valid SSLConfig while there is no easy way for 
them to get a default-not-enabled one gives the caller unnecessary burden. We 
can either allow null here, or have SSLConfigurationFactory add a method to 
return a default-nothing-enabled config?


- Jinmei


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52488/#review151207
---


On Oct. 3, 2016, 5:52 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52488/
> ---
> 
> (Updated Oct. 3, 2016, 5:52 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, Kirk Lund, and Udo 
> Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-420: fix Pulse test when not using any SSLConfig
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/JettyHelper.java
>  089dbacb056da84d6d6ceb36dad6f09b9ce73890 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/testbed/driver/PulseUITest.java
>  a2365a2dfa3e221fa50664c3f8d96fae2bd93c86 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAbstractTest.java
>  5024250f8ff3867e0f527d7ef5fbbd501e2d64ee 
> 
> Diff: https://reviews.apache.org/r/52488/diff/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Review Request 52488: GEODE-420: fix Pulse test when not using any SSLConfig

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52488/
---

Review request for geode, Jared Stewart, Kevin Duling, Kirk Lund, and Udo 
Kohlmeyer.


Repository: geode


Description
---

GEODE-420: fix Pulse test when not using any SSLConfig


Diffs
-

  
geode-core/src/main/java/org/apache/geode/management/internal/JettyHelper.java 
089dbacb056da84d6d6ceb36dad6f09b9ce73890 
  
geode-pulse/src/test/java/org/apache/geode/tools/pulse/testbed/driver/PulseUITest.java
 a2365a2dfa3e221fa50664c3f8d96fae2bd93c86 
  
geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAbstractTest.java
 5024250f8ff3867e0f527d7ef5fbbd501e2d64ee 

Diff: https://reviews.apache.org/r/52488/diff/


Testing
---

precheckin running


Thanks,

Jinmei Liao

Re: IntelliJ Code Style XML

[Jinmei Liao]

It's under open/etc

On Wed, Sep 28, 2016 at 10:17 AM, Jared Stewart  wrote:

> From where can I obtain the intellijIdeaCodeStyle.xml file referenced in
> the wiki page  Style+Guide> about code style?
>
> Thanks,
> Jared




-- 
Cheers

Jinmei

Re: Review Request 52306: GEODE-1769: add the ignored exception

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52306/
---

(Updated Sept. 27, 2016, 6:13 p.m.)


Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.


Repository: geode


Description
---

GEODE-1769: add the ignored exception


Diffs (updated)
-

  
geode-core/src/test/java/org/apache/geode/security/AbstractSecureServerDUnitTest.java
 23b851a3c0c6c8556ca3a0d0cb65e5583c2c38d5 
  
geode-core/src/test/java/org/apache/geode/security/IntegratedClientRegisterInterestAuthDistributedTest.java
 f897b5e27cc63d22295df6b7f7596c9265929e75 

Diff: https://reviews.apache.org/r/52306/diff/


Testing
---

the affected test


Thanks,

Jinmei Liao

Review Request 52306: GEODE-1769: add the ignored exception

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52306/
---

Review request for geode.


Repository: geode


Description
---

GEODE-1769: add the ignored exception


Diffs
-

  
geode-core/src/test/java/org/apache/geode/security/IntegratedClientRegisterInterestAuthDistributedTest.java
 f897b5e27cc63d22295df6b7f7596c9265929e75 

Diff: https://reviews.apache.org/r/52306/diff/


Testing
---

the affected test


Thanks,

Jinmei Liao

Re: Review Request 52269: GEODE-1659: put security properties in the cluster config and applied to all the members in the cluster.

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52269/
---

(Updated Sept. 27, 2016, 5:25 p.m.)


Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.


Repository: geode


Description (updated)
---

GEODE-1659: put security properties in the cluster config and applied to all 
the members in the cluster.

When joining a secured server, the server's security-manager and 
security-post-processor should match whatever the cluster config is. 
Also prevent a member not using cluster configuration from joining a secured 
cluster.

broke down requestAndApplySharedConfiguration into individual steps so that we 
can apply properties and cache.xml in different phases of cache initialization


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/distributed/internal/SharedConfiguration.java
 2e6677f3dff5bc9a811185e9844b22e4ed925971 
  
geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java
 5281e117d2ab4c67eb0db13490e6c21aff74f5c5 
  
geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java 
93a9c3b1686a82d8c7814fbda47650d4a26ebbd2 
  geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java 
d341f51f493dc4e0391be26b15d64d7568c35fcb 
  
geode-core/src/main/java/org/apache/geode/management/internal/SystemManagementService.java
 c42d1a259e21a02a4aee0dc3fd4c1a51779b79e3 
  
geode-core/src/test/java/org/apache/geode/internal/cache/GemFireCacheImplTest.java
 597a977ccaab900fe38a4042b996daedaef99456 
  
geode-core/src/test/java/org/apache/geode/security/PDXGfshPostProcessorOnRemoteServerTest.java
 34043e8c894ae7a209b666841254e620cdd2b158 
  
geode-core/src/test/java/org/apache/geode/security/SecurityClusterConfigDUnitTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/security/SecurityWithoutClusterConfigDUnitTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java
 8468664446a3953bf5dfb056621bfcbdd0c8daa4 

Diff: https://reviews.apache.org/r/52269/diff/


Testing
---

precheckin running


Thanks,

Jinmei Liao

Review Request 52269: GEODE-1659: put security properties in the cluster config and applied to all the members in the cluster.

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52269/
---

Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.


Repository: geode


Description
---

GEODE-1659: put security properties in the cluster config and applied to all 
the members in the cluster.


Diffs
-

  
geode-core/src/main/java/org/apache/geode/distributed/internal/SharedConfiguration.java
 2e6677f3dff5bc9a811185e9844b22e4ed925971 
  
geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java 
93a9c3b1686a82d8c7814fbda47650d4a26ebbd2 
  geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java 
d341f51f493dc4e0391be26b15d64d7568c35fcb 
  
geode-core/src/main/java/org/apache/geode/management/internal/SystemManagementService.java
 c42d1a259e21a02a4aee0dc3fd4c1a51779b79e3 
  
geode-core/src/test/java/org/apache/geode/security/SecurityClusterConfigDUnitTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/security/SecurityWithoutClusterConfigDUnitTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java
 8468664446a3953bf5dfb056621bfcbdd0c8daa4 

Diff: https://reviews.apache.org/r/52269/diff/


Testing
---

precheckin running


Thanks,

Jinmei Liao

security properties in the cluster config

[Jinmei Liao]

Hi, All,

I am working on this ticket:
https://issues.apache.org/jira/browse/GEODE-1659. Basically, currently, any
member(locator or server) needs to specify its own security-manager in
order to protect its data which could leads to misconfiguration and data
leak. So we would like to put it into the cluster configuration so any
member who wants to join the cluster will need to apply the same security
measures.

Now Here is my question, should we only put the "security-manager" and
"security-post-processor" in the cluster config or any "security-*"
settings, which include SSL settings as well.

Thanks!

-- 
Cheers

Jinmei

Re: Review Request 52067: GEODE-1909: add authorization in GMSAuthenticator

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52067/
---

(Updated Sept. 19, 2016, 9:29 p.m.)


Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.


Repository: geode


Description
---

GEODE-1909: add authorization in GMSAuthenticator

The user who get authenticated in the cluster needs to have CLUSTER:MANAGE 
previlage as well. Note this is NOT the user who issued "start server" command 
in gfsh, but is the peer user whose credential is put in the server's 
security.properties file.


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberFactory.java
 08178e92ad9b4db30534c80873c63e8b4c11808f 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java
 3f030c9af4ab0503ad201c9172fe61a256eb961a 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
 d15e8bfa09b3c24d8c2db10d524bfce0d4b61ad5 
  geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java 
b61201ef60b4dc3ea6d42e7ae30d305e2ebb7e35 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticatorWithAuthenticatorTest.java
 45a5881207179e950bb40a897660f63f583a4e19 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticatorWithSecurityManagerTest.java
 ddb208d043a8503b52119e363ff23c54acd0a4f4 
  
geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java
 PRE-CREATION 
  
geode-core/src/test/resources/org/apache/geode/management/internal/security/cacheServer.json
 3bb3e2f010da9e97af3ade60dbd4b4b45b9370b3 

Diff: https://reviews.apache.org/r/52067/diff/


Testing
---

precheckin running


Thanks,

Jinmei Liao

Review Request 52067: GEODE-1909: add authorization in GMSAuthenticator

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52067/
---

Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.


Repository: geode


Description
---

GEODE-1909: add authorization in GMSAuthenticator

The user who get authenticated in the cluster needs to have CLUSTER:MANAGE 
previlage as well. Note this is NOT the user who issued "start server" command 
in gfsh, but is the peer user whose credential is put in the server's 
security.properties file.


Diffs
-

  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMemberFactory.java
 08178e92ad9b4db30534c80873c63e8b4c11808f 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java
 3f030c9af4ab0503ad201c9172fe61a256eb961a 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
 d15e8bfa09b3c24d8c2db10d524bfce0d4b61ad5 
  geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java 
b61201ef60b4dc3ea6d42e7ae30d305e2ebb7e35 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticatorWithAuthenticatorTest.java
 45a5881207179e950bb40a897660f63f583a4e19 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticatorWithSecurityManagerTest.java
 ddb208d043a8503b52119e363ff23c54acd0a4f4 
  
geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java
 PRE-CREATION 
  
geode-core/src/test/resources/org/apache/geode/management/internal/security/cacheServer.json
 3bb3e2f010da9e97af3ade60dbd4b4b45b9370b3 

Diff: https://reviews.apache.org/r/52067/diff/


Testing
---

precheckin running


Thanks,

Jinmei Liao

Re: [GitHub] incubator-geode pull request #240: GEODE-1899: Renamed the custom 'clean' ta...

[Jinmei Liao]

Looks harmless. I'll pull this in. Thanks!

On Thu, Sep 15, 2016 at 2:44 PM, jaredjstewart  wrote:

> GitHub user jaredjstewart opened a pull request:
>
> https://github.com/apache/incubator-geode/pull/240
>
> GEODE-1899: Renamed the custom 'clean' task to work with Gradle 3.0+
>
>
>
> You can merge this pull request into a Git repository by running:
>
> $ git pull https://github.com/jaredjstewart/incubator-geode
> feature/GEODE-1899
>
> Alternatively you can review and apply these changes as the patch at:
>
> https://github.com/apache/incubator-geode/pull/240.patch
>
> To close this pull request, make a commit to your master/trunk branch
> with (at least) the following in the commit message:
>
> This closes #240
>
> 
> commit 804a1cabc78357bea13844dd767afcc66a7426af
> Author: Jared Stewart 
> Date:   2016-09-15T20:24:34Z
>
> GEODE-1899: Renamed the custom 'clean' task to work with Gradle 3.0+
>
> 
>
>
> ---
> If your project is set up for it, you can reply to this email and have your
> reply appear on GitHub as well. If your project does not have this feature
> enabled and wishes so, or if the feature is enabled but not working, please
> contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
> with INFRA.
> ---
>



-- 
Cheers

Jinmei

Re: Review Request 51926: GEODE-1898: fix geode-core:javadoc warnings

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51926/#review149129
---


Ship it!




Ship It!

- Jinmei Liao


On Sept. 15, 2016, 6:27 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51926/
> ---
> 
> (Updated Sept. 15, 2016, 6:27 p.m.)
> 
> 
> Review request for geode, Jinmei Liao and Kevin Duling.
> 
> 
> Bugs: GEODE-1898
> https://issues.apache.org/jira/browse/GEODE-1898
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1898: fix geode-core:javadoc warnings
> 
> Remove unused javadoc tags because they generate warnings during gradle 
> javadoc task.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
>  2ed4646 
>   geode-core/src/main/java/org/apache/geode/internal/tcp/TCPConduit.java 
> 3cdfa5a 
> 
> Diff: https://reviews.apache.org/r/51926/diff/
> 
> 
> Testing
> ---
> 
> clean build and javadoc
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Re: Review Request 51923: GEODE-1883: making AuthInit optional when starting a server/client

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51923/
---

(Updated Sept. 15, 2016, 9 p.m.)


Review request for geode, Jared Stewart, Kevin Duling, Kirk Lund, and Swapnil 
Bawaskar.


Repository: geode


Description
---

GEODE-1883: making AuthInit optional when starting a server/client

When starting a server trying to connect to a locator, customer should not have 
to implement an AuthInit object, they can just put 
security-username/security-password in their properties file.


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
 8a4446cc19e5cdc0685e5bd6b2708d7da4f0d101 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java
 68ec0c0041f204775541db396022a1df14c868fe 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/HandShake.java
 00372aebc8d72376ff6613c13637faeaac8d1523 
  
geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
 7380c9a0cb7382f8a2758acc1018fd793ce815df 
  geode-core/src/main/java/org/apache/geode/security/AuthInitialize.java 
9123ec4ac00eb2fe5f7d980e78c06c6fe6bcd767 
  
geode-core/src/test/java/org/apache/geode/security/PDXGfshPostProcessorOnRemoteServerTest.java
 870ff91ab1a3d3537754268f6e2f2c33f24141b7 

Diff: https://reviews.apache.org/r/51923/diff/


Testing
---

precheckin


Thanks,

Jinmei Liao

Review Request 51925: GEODE-1648: revert commits related to security-enabled-components.

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51925/
---

Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.


Repository: geode


Description
---

* revert this if we want to pull this back in.

We decided to use an all or nothing approach for security.


Diffs
-

  
geode-core/src/main/java/org/apache/geode/cache/client/internal/ConnectionFactoryImpl.java
 b6460ebc35d0a7e8f2bfb6838249d1f6153d92d6 
  
geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
 d843792168e91b39b686c84de66b0087c1ae65b4 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java
 727c5ab504aadb950b5f765a7201e0704107f9bc 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfig.java
 692c2b9aa4269cc332b8a833af80b69cfd8404e4 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfigImpl.java
 5a3ec274f1209d6c71c923e74b60c1f2edea0aaf 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java
 68ec0c0041f204775541db396022a1df14c868fe 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/fd/GMSHealthMonitor.java
 5717c30c8234452857a0226207663d13b72e7dd3 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
 495a85b8403be543f619b4d3cf88b5100d1ef159 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java
 bd6a8f8a428ff30cc532788f6f57ed37585f06a9 
  geode-core/src/main/java/org/apache/geode/internal/admin/SSLConfig.java 
4b96d5519f0b75fff3937f04e8cb2404859c3970 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
 5bddfa5a9b2afc7f1746bad69eee3048c7f32c9e 
  geode-core/src/main/java/org/apache/geode/internal/net/SocketCreator.java 
c6ad9ce3000842f7d6b4a34389d02123e0249af3 
  
geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
 2ed46465e12633b794458c9de6630ae037a631d1 
  
geode-core/src/main/java/org/apache/geode/internal/security/SecurableComponent.java
 1eac87c80ad2c64021362311c5d369c198af209a 
  
geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
 7380c9a0cb7382f8a2758acc1018fd793ce815df 
  geode-core/src/main/java/org/apache/geode/internal/tcp/TCPConduit.java 
3cdfa5a82ad23df5b6f0992ad0781bd71dd248fd 
  geode-core/src/main/java/org/apache/geode/management/GemFireProperties.java 
592bfdd982e15d3a5379dc3648d984b28ea8cf7b 
  
geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java
 ad4b3b746ce7e3ea40f4ccfcd6b103e37eb1f16c 
  geode-core/src/main/java/org/apache/geode/security/SecurableComponents.java 
beb560086f164ea8a02bdc3e379eace3f44ff599 
  
geode-core/src/test/java/org/apache/geode/cache/client/internal/CacheServerSSLConnectionDUnitTest.java
 e1ee4b104b111702d551078e7e145e95349dc498 
  
geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherRemoteIntegrationTest.java
 2aa0c7d74831bcbcfbe5e87bfc75b9710d9553fa 
  
geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherRemoteIntegrationTest.java
 98ee86f0bb69e8e655e5fa4860413d44b5465149 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/AbstractDistributionConfigTest.java
 293cbd2d6e0c10fac32175d324cb7048cd9057b5 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java
 978a0d021c6efb055fc4a3233e9150781ce54b7a 
  
geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceTest.java
 333875edd3af22c8b39c4e1f2363cf90f4a1c754 
  
geode-core/src/test/java/org/apache/geode/internal/security/SecurityConfigIntegrationTest.java
 aab934e6521e6a203ca839950afe4cf0b53737e7 
  
geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java
 1bf1056ccff1f1d2e9f254102545fe82fd18b1a2 
  geode-core/src/test/java/org/apache/geode/management/JMXMBeanDUnitTest.java 
05c9022e36e50261a2c83548d092b9b46b8f23ff 
  
geode-core/src/test/java/org/apache/geode/security/IntegratedSecurityCacheLifecycleDistributedTest.java
 494c4d46cdce885a39e618bdac2f9a874e530194 
  
geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt 
b3bab0f4d9e6d86d5c86506c6d4893f444f32e27 

Diff: https://reviews.apache.org/r/51925/diff/


Testing
---

precheckin


Thanks,

Jinmei Liao

Review Request 51923: GEODE-1883: making AuthInit optional when starting a server/client

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51923/
---

Review request for geode, Jared Stewart, Kevin Duling, Kirk Lund, and Swapnil 
Bawaskar.


Repository: geode


Description
---

GEODE-1883: making AuthInit optional when starting a server/client

When starting a server trying to connect to a locator, customer should not have 
to implement an AuthInit object, they can just put 
security-username/security-password in their properties file.


Diffs
-

  
geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
 d843792168e91b39b686c84de66b0087c1ae65b4 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/auth/GMSAuthenticator.java
 68ec0c0041f204775541db396022a1df14c868fe 
  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/HandShake.java
 00372aebc8d72376ff6613c13637faeaac8d1523 
  
geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
 7380c9a0cb7382f8a2758acc1018fd793ce815df 
  geode-core/src/main/java/org/apache/geode/security/AuthInitialize.java 
9123ec4ac00eb2fe5f7d980e78c06c6fe6bcd767 
  
geode-core/src/test/java/org/apache/geode/security/PDXGfshPostProcessorOnRemoteServerTest.java
 870ff91ab1a3d3537754268f6e2f2c33f24141b7 

Diff: https://reviews.apache.org/r/51923/diff/


Testing
---

precheckin


Thanks,

Jinmei Liao

Re: securing geode components

[Jinmei Liao]

That is my original thought as well. If we are protecting resources
(CLUSTER and DATA), it should be protected no matter which way user is
trying to access it. I guess I'll leave this to the PMs to decide.

On Thu, Sep 8, 2016 at 7:37 PM, Anthony Baker  wrote:

> Udo, Kirk - this makes sense and thanks for the discussion to help clarify
> the issue.
>
> Regarding GEODE-1648, does it make sense to do at all?  That is, what if
> role-based access control is either ON | OFF instead of per component.  If
> we allow disabling RBAC for certain components, wouldn’t that make it
> possible to create a backdoor?  Could we start with a binary option and
> enable more granular control when requested by users?
>
> Anthony
>
> > On Sep 8, 2016, at 4:11 PM, Kirk Lund  wrote:
> >
> > +1 overall with some feedback...
> >
> > 1) I think the list is reasonable with a few nitpicks below
> >
> > 2) If these are Channels and not Components, then I would probably name
> it
> > SecurableChannels or SecurableCommunicationChannels or whatever.
> >
> > 3) I'd prefer HTTP be renamed to Web or other non-protocol word -- HTTP
> is
> > a protocol and the names of the other channels are conceptual channels
> > rather than a protocol (the others use TCP/IP or RMI but we don't label
> > them as such). Or am I missing something here?
> >
> > 4) JMX is probably ok. Currently we are using (and securing) JMX over RMI
> > (javax.management.remote.rmi.RMIConnectorServer). There are other
> > connectors for JMX including HTTP (ex: mx4j.tools.adaptor.http.
> HttpAdaptor)
> > and SNMP (ex: com.sun.jmx.snmp.daemon.SnmpAdaptorServer). We only need
> JMX
> > over RMI for now, but would we add those others as new enums to
> > SecurableChannels later if we add anything like that to Geode? Or would
> we
> > try to group those all together under the name JMX? Or decide when the
> time
> > comes?
> >
> > I think we should try to steer away from being overly controlled by specs
> > especially for reasonable changes. We all follow agile process, so a
> > decision made one iteration could easily be undone or changed in the next
> > iteration and most of us are following weekly iterations.
> >
> > After a release anything exposed in a User API is very difficult to
> change
> > due to backwards compatibility constraints. I think we should be much
> more
> > careful with User APIs in Geode going forward to avoid some of the
> problems
> > we have with pre-existing Geode User APIs that we inherited.
> >
> > -Kirk
> >
> > On Thu, Sep 8, 2016 at 2:07 PM, Udo Kohlmeyer 
> wrote:
> >
> >> As GEODE-420 deals with SSL comms configuration and GEODE-1648 with
> >> Authentication I think we need to be careful in what is
> >> feasible and what is logical.
> >>
> >> For SSL comms it was decided that the following components are relevant
> >> [1]  >> SL+properties>:
> >>
> >> * Locator => The comms channel between Locators + the initial comms
> >>   channel between clients and locator
> >> * Cluster => Internode comms channel (peer to peer)
> >> * Server => Client-server comms channel
> >> * Gateway => Comms channel between WAN Gateway senders/receivers
> >> * HTTP => Any HTTP comms. incl REST and Pulse
> >> * JMX => Any JMX comms
> >>
> >> These components were selected as they seem to be logical boundaries and
> >> communication interfaces.
> >>
> >> I think the specialization of HTTP, for Authentication are
> >> functions of those interfaces:
> >>
> >> * REST-admin
> >> * REST-dev
> >> * Pulse
> >>
> >> I think that comms and functions exposed by those comms should not be
> >> mixed. I think that securing the comms channel is a factor of "trust".
> Do I
> >> implicitly trust the interface/system that I am connected to or are
> >> connecting to.
> >>
> >> I think concepts like "management" is a concept in function. Do I allow
> a
> >> user to access admin API's? The function of management should not
> determine
> >> if a system trusts another systems connection. When a new comms
> interface
> >> is added (say messaging), we want to be able to trust that comms
> channel.
> >> The "management" function should still work regardless of interface, be
> it
> >> jmx,http/rest,prop tcp,messaging.
> >>
> >> --Udo
> >>
> >> [1]: https://cwiki.apache.org/confluence/display/GEODE/Revised+SS
> >> L+properties
> >>
> >>
> >>
> >> On 9/09/2016 5:49 AM, Swapnil Bawaskar wrote:
> >>
> >>> GEODE-1648 and GEODE-420 are both trying to add geode properties to
> secure
> >>> only some components.
> >>> GEODE-1648 is intending to add a property named
> >>> "security-enabled-components" that will allow users to turn off
> >>> authentication/authorization for some components
> >>> GEODE-420 is intending to add a property named "ssl-enabled-components"
> >>> that will allow users to turn off ssl. for either client/server,
> >>> peer-to-peer or wan communication.
> >>>
> >>>