[GitHub] geode pull request #577: Feature/geode 3049

[nreich]

Github user nreich commented on a diff in the pull request:

https://github.com/apache/geode/pull/577#discussion_r121828113
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/cache/BucketRedundancyTracker.java
 ---
@@ -0,0 +1,108 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for 
additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF 
ANY KIND, either express
+ * or implied. See the License for the specific language governing 
permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.internal.cache;
+
+/**
+ * Keeps track of redundancy status for a PartitionedRegion and (if 
enabled) updates the statistics
+ * for the region.
+ */
+class BucketRedundancyTracker {
+  private boolean redundancySatisfied = false;
--- End diff --

Based on our conversation, I am making the methods (other than getting the 
current status) synchronized to protect the data, instead of relying on any 
outside synchronization.


---
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.
---

Please Update or Verify Your IDE Style Scheme.

[Patrick S Rhomberg]

Hello, all.

A recent update to the develop branch (a561bd...) resolves the disagreement
between IntelliJ and Eclipse import ordering.  The IntelliJ style xml
present in geode/etc was inconsistent with the Eclipse import ordering file
located in the same directory.

IntelliJ users, please update your style scheme to the now-canonical import
ordering.  This can be done through Preferences: Editor > Code Style >
Java: Scheme.

Eclipse users, please verify that your IDE also respects the import
ordering specified by
geode/etc/eclipseOrganizeImports.importorder.

Thank you.

[Spring CI] Spring Data GemFire > Nightly-ApacheGeode > #584 was SUCCESSFUL (with 1868 tests)

[Spring CI]

---
Spring Data GemFire > Nightly-ApacheGeode > #584 was successful.
---
Scheduled
1870 tests in total.

https://build.spring.io/browse/SGF-NAG-584/





--
This message is automatically generated by Atlassian Bamboo

Fwd: [GitHub] geode pull request #569: Resolve Eclipse/IntelliJ import order disagreement

[Jinmei Liao]

This is just pulled on to develop to resolve the import order difference
between intelliJ and Eclipse.

For IntelliJ users, please go to your settings->Code Style->Java and click
the little gear symbol beside the "Scheme" box to import the new intellJ
sytle xml into your IDEA, the xml is in under open/etc/ folder.

-- Forwarded message --
From: asfgit 
Date: Tue, Jun 13, 2017 at 3:13 PM
Subject: [GitHub] geode pull request #569: Resolve Eclipse/IntelliJ import
order disagreement
To: dev@geode.apache.org


Github user asfgit closed the pull request at:

https://github.com/apache/geode/pull/569


---
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: [DISCUSS] Using Pull Requests over Review Board

[Jacob Barrett]

On Mon, Jun 12, 2017 at 9:37 AM Dave Barnes  wrote:

> Proposal as written says "...for changes that would require peer review
> before committing...".
> If this means that minor changes (in my case, typo repairs are the common
> case) can be checked in without going through a PR process, I can go with
> the group decision.
>

The proposal only changes the method for items that would have normally
gone through a review board. It isn't proposing a change to the items that
are reviewed.

Re: [DISCUSS] Using Pull Requests over Review Board

[Jacob Barrett]

On Mon, Jun 12, 2017 at 7:57 AM Bruce Schuchardt 
wrote:

> It places an unnecessary burden on committers


Considering committers need to use PR to commit changes from non-committers
how does reducing the number of review systems increase the burden on
committers?


> and git history is the
> definitive record of changes to the code so github pr history isn't
> really very useful.


All unsquashed commit history is preserved through a PR.

Re: Review Request 60051: GEODE-2301: Deprecate JTA TransactionManagerImpl

[Darrel Schneider]

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



All of these changes are to classes in an "internal" package. Do we not have 
any external classes/interfaces to deprecate?

- Darrel Schneider


On June 13, 2017, 9:53 a.m., Eric Shu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60051/
> ---
> 
> (Updated June 13, 2017, 9:53 a.m.)
> 
> 
> Review request for geode, anilkumar gingade, Darrel Schneider, Lynn Gallinat, 
> and Swapnil Bawaskar.
> 
> 
> Bugs: GEODE-2301
> https://issues.apache.org/jira/browse/GEODE-2301
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> User should use third-party JTA transaction manager.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/jta/GlobalTransaction.java 
> 03eeb20 
>   geode-core/src/main/java/org/apache/geode/internal/jta/TransactionImpl.java 
> a5e80b6 
>   
> geode-core/src/main/java/org/apache/geode/internal/jta/TransactionManagerImpl.java
>  15ab1f8 
>   
> geode-core/src/main/java/org/apache/geode/internal/jta/UserTransactionImpl.java
>  2471e02 
>   geode-core/src/main/java/org/apache/geode/internal/jta/XidImpl.java 865f0aa 
> 
> 
> Diff: https://reviews.apache.org/r/60051/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Eric Shu
> 
>

Re: Review Request 60051: GEODE-2301: Deprecate JTA TransactionManagerImpl

[Darrel Schneider]

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




geode-core/src/main/java/org/apache/geode/internal/jta/GlobalTransaction.java
Lines 25 (patched)


Change "GEODE" to "Geode".
Change "User" to "user".
Change "use third" to "use a third".


- Darrel Schneider


On June 13, 2017, 9:53 a.m., Eric Shu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60051/
> ---
> 
> (Updated June 13, 2017, 9:53 a.m.)
> 
> 
> Review request for geode, anilkumar gingade, Darrel Schneider, Lynn Gallinat, 
> and Swapnil Bawaskar.
> 
> 
> Bugs: GEODE-2301
> https://issues.apache.org/jira/browse/GEODE-2301
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> User should use third-party JTA transaction manager.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/jta/GlobalTransaction.java 
> 03eeb20 
>   geode-core/src/main/java/org/apache/geode/internal/jta/TransactionImpl.java 
> a5e80b6 
>   
> geode-core/src/main/java/org/apache/geode/internal/jta/TransactionManagerImpl.java
>  15ab1f8 
>   
> geode-core/src/main/java/org/apache/geode/internal/jta/UserTransactionImpl.java
>  2471e02 
>   geode-core/src/main/java/org/apache/geode/internal/jta/XidImpl.java 865f0aa 
> 
> 
> Diff: https://reviews.apache.org/r/60051/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Eric Shu
> 
>

Re: Review Request 60028: GEODE-2632: consolidate different types of SecurityService

[Jinmei Liao]

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

(Updated June 13, 2017, 9:28 p.m.)


Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
Patrick Rhomberg.


Changes
---

test update
this changeset was started when I was rebasing my finer graned security on top 
of Kirk's change and I found out that I had to implemented the newly added 
interaface methods in all the implemtations and introduce more duplicate codes. 
I do believe this changeset makes some improvement, but if the team thinks this 
is not necessary, I am happy to bury it.


Repository: geode


Description (updated)
---

* combine EnabledSecurityService and CustomSecurityService into 
IntegratedSecurityService
* combine DisabledSecurityService and LegacySecurityService
* combine ConfigInitalizer and RealmInitilizer
* provide default impelementations of SecurityService
* consolidate SecurityService creation.


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
 22edb6f06c7791929cc9a033ca1a1bfed5751a47 
  
geode-core/src/main/java/org/apache/geode/internal/security/CallbackInstantiator.java
 3ff632d3857189513243959ee96da89da66d5a27 
  
geode-core/src/main/java/org/apache/geode/internal/security/CustomSecurityService.java
 0ba1cb62468f15fda60a94dac9c781fe1793b28f 
  
geode-core/src/main/java/org/apache/geode/internal/security/DisabledSecurityService.java
 b50569036db1acac927f08ee5c1771c72ede3c1d 
  
geode-core/src/main/java/org/apache/geode/internal/security/EnabledSecurityService.java
 f0568c0ebc2b17c47b5059c60aa4f8bd500c5d6c 
  
geode-core/src/main/java/org/apache/geode/internal/security/LegacySecurityService.java
 44562537bc426e47a42d680bb410dc59bf9bd854 
  
geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
 a4041e198f1a9a4961915504c51256d0b03aa7c2 
  
geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceFactory.java
 02f34f15617f7bf4ad9ee7fa51f32be4db3c198a 
  
geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceType.java
 8ae76d22b628b3175db45116b80dfcfbe34aba1d 
  
geode-core/src/main/java/org/apache/geode/internal/security/shiro/ConfigInitializer.java
 60f014b9c33732a4ea134a654e666a9439b210bb 
  
geode-core/src/main/java/org/apache/geode/internal/security/shiro/CustomAuthRealm.java
 51449fdd5570494f3cf91325985a5eb9fc9f6d57 
  
geode-core/src/main/java/org/apache/geode/internal/security/shiro/RealmInitializer.java
 978c4dd0ab92afde53972f7feb9d8f018d0bf662 
  
geode-core/src/main/java/org/apache/geode/internal/security/shiro/ShiroSecurityManagerInitializer.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/membership/MembershipJUnitTest.java
 a0c3cf3074051990cc50755131f8024db0b1faad 
  
geode-core/src/test/java/org/apache/geode/internal/security/DisabledSecurityServiceTest.java
 cacbeed957c3b87d08c93db74e38e0134565f699 
  
geode-core/src/test/java/org/apache/geode/internal/security/EnabledSecurityServiceTest.java
 fca7eae9413cee98d351db5349fd950d3aa56180 
  
geode-core/src/test/java/org/apache/geode/internal/security/FakePostProcessor.java
 70823443b5c4f776c86bb28ed49a73e690c5c872 
  
geode-core/src/test/java/org/apache/geode/internal/security/FakeSecurityManager.java
 ca4e6b7fb462bb4e3fefbc5db8a9503c6b13a865 
  
geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceConstructorTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/internal/security/LegacySecurityServiceTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryShiroIntegrationTest.java
 89070123978c22c4cfa8684fbb5b033dc9d83ffa 
  
geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryTest.java
 f027a4367b38681f83dad2d4c4add67759633644 
  
geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceTest.java
 44893520962331bcd41e972afa661538c28d4fb2 
  
geode-core/src/test/java/org/apache/geode/internal/security/shiro/ConfigInitializerIntegrationTest.java
 857c0be8940b4acde2aa4992fac0122b687391c2 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithCustomRealmIntegrationTest.java
 01d6bb6488e76fb3cf652ad211e9f7e2fac51389 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithShiroIniIntegrationTest.java
 1caedbcede239d6a96640381cc6941948637b442 
  
geode-core/src/test/java/org/apache/geode/security/CacheFactoryWithSecurityObjectTest.java
 cdb90f1b580edaf6a2762883d4159a45d69c4728 
  
geode-core/src/test/java/org/apache/geode/security/SecurityManagerLifecycleDistributedTest.java
 a9048b9219a494f61e3873ee3f2908da04bf6154 
  

[GitHub] geode pull request #578: GEODE-1958: Removing/deprecating PasswordUtil

[PurelyApplied]

Github user PurelyApplied commented on a diff in the pull request:

https://github.com/apache/geode/pull/578#discussion_r121799720
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/SystemAdmin.java ---
@@ -1909,10 +1900,7 @@ private static boolean needsSysDir(String cmd) {
 if (cmd.equalsIgnoreCase("version")) {
   return false;
 }
-if (cmd.equalsIgnoreCase("help")) {
-  return false;
-}
-return true;
+return !cmd.equalsIgnoreCase("help");
--- End diff --

If you want to collapse this block, I think this might read a little better.

```
if (cmd.equalsIgnoreCase("stats")
|| cmd.equalsIgnoreCase("merge-logs")
|| cmd.equalsIgnoreCase("version")
|| cmd.equalsIgnoreCase("help")) {
return false;
}
return true;
```

I guess you could collapse that to a one-liner, but I don't think that 
would read as well.


---
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.
---

[GitHub] geode pull request #578: GEODE-1958: Removing/deprecating PasswordUtil

[PurelyApplied]

Github user PurelyApplied commented on a diff in the pull request:

https://github.com/apache/geode/pull/578#discussion_r121798399
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/util/PasswordUtil.java ---
@@ -44,72 +42,29 @@
   private static byte[] init = "string".getBytes();
 
   /**
-   * Encrypts a password string
-   * 
-   * @param password String to be encrypted.
-   * @return String encrypted String
-   */
-  public static String encrypt(String password) {
-return encrypt(password, true);
-  }
-
-  /**
-   * 
-   * @param password String to be encrypted
-   * @param echo if true prints result to system.out
-   * @return String encrypted String
+   * Decrypts an encrypted password string.
+   *
+   * @param password String to be decrypted
+   * @return String decrypted String
*/
-  public static String encrypt(String password, boolean echo) {
-String encryptedString = null;
+  @Deprecated
+  public static String decrypt(String password) {
+String toDecrypt;
+if (password.startsWith("encrypted(") && password.endsWith(")"))
--- End diff --

I don't think it gets caught by spotless, but officially we prefer to wrap 
every `if` and `else` in curly braces, even when they're just one-liners.


---
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.
---

Re: Review Request 60052: GEODE-2294: revert to avoid rest protocol change

[Jared Stewart]

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


Ship it!




Ship It!

- Jared Stewart


On June 13, 2017, 5:29 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60052/
> ---
> 
> (Updated June 13, 2017, 5:29 p.m.)
> 
> 
> Review request for geode, Anthony Baker, Emily Yeh, Jared Stewart, Ken Howe, 
> Kirk Lund, and Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2294: revert to avoid rest protocol change
> 
> 
> Diffs
> -
> 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/AbstractBaseController.java
>  d8eb5721315b65f2924a54e565a1dad2815a5afa 
>   
> geode-web-api/src/test/java/org/apache/geode/rest/internal/web/controllers/AbstractBaseControllerJUnitTest.java
>  7ebfbcfe9d97a2e984b2b9975b95945971d4b1ef 
> 
> 
> Diff: https://reviews.apache.org/r/60052/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 60025: GEODE-3060: Introduce JUnit rule for testing the fully-assembled GFSH

[Patrick Rhomberg]

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




geode-assembly/src/test/java/org/apache/geode/test/dunit/rules/gfsh/GfshRule.java
Lines 54 (patched)


Just as a point of order...

You convert the`IOException` to a `RuntimeException` so that (a) tests 
using this rule won't have to explicitly catch the `IOException`, and (b) since 
it's just tests, it's okay to explode all the way up with the 
`RuntimeException`?



geode-assembly/src/test/java/org/apache/geode/test/dunit/rules/gfsh/GfshScript.java
Lines 83-93 (patched)


Is there a use case, maybe as we expand to acceptance testing and try to 
engineer some race conditions, where we might want to run a script with some 
expected exit value but not necessarily wait for it to finish?

My thought is that if you want to quickly spawn two `GfshScript`s with 
different expected values, you might give them expected exit values but not 
wait times, causing an `IllegalStateException` in line 91.

Maybe additional methods `checkExitCode` and `checkExitCodeAfterWaiting` or 
something would help?  I don't know.  I could explain this in person better...


- Patrick Rhomberg


On June 12, 2017, 9:50 p.m., Jared Stewart wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60025/
> ---
> 
> (Updated June 12, 2017, 9:50 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-3060: Introduce JUnit rule for testing the fully-assembled GFSH
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StatusLocatorRealGfshTest.java
>  PRE-CREATION 
>   
> geode-assembly/src/test/java/org/apache/geode/test/dunit/rules/gfsh/GfshRule.java
>  PRE-CREATION 
>   
> geode-assembly/src/test/java/org/apache/geode/test/dunit/rules/gfsh/GfshScript.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60025/diff/2/
> 
> 
> Testing
> ---
> 
> Precheckin started (still running)
> 
> 
> Thanks,
> 
> Jared Stewart
> 
>

Re: Review Request 60028: GEODE-2632: consolidate different types of SecurityService

[Jinmei Liao]

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

(Updated June 13, 2017, 8:26 p.m.)


Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
Patrick Rhomberg.


Changes
---

consolidating the ConfigInitializer and RealmInitanilizer into 
ShiroSecurityManagerInitilizer. This helps UnitTset mock and also further 
simplify the code in SecurityServiceFactory.


Repository: geode


Description
---

* combine EnabledSecurityService and CustomSecurityService into 
IntegratedSecurityService
* combine DisabledSecurityService and LegacySecurityService
* provide default impelementations of SecurityService
* consolidate SecurityService creation.


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
 22edb6f06c7791929cc9a033ca1a1bfed5751a47 
  
geode-core/src/main/java/org/apache/geode/internal/security/CallbackInstantiator.java
 3ff632d3857189513243959ee96da89da66d5a27 
  
geode-core/src/main/java/org/apache/geode/internal/security/CustomSecurityService.java
 0ba1cb62468f15fda60a94dac9c781fe1793b28f 
  
geode-core/src/main/java/org/apache/geode/internal/security/DisabledSecurityService.java
 b50569036db1acac927f08ee5c1771c72ede3c1d 
  
geode-core/src/main/java/org/apache/geode/internal/security/EnabledSecurityService.java
 f0568c0ebc2b17c47b5059c60aa4f8bd500c5d6c 
  
geode-core/src/main/java/org/apache/geode/internal/security/LegacySecurityService.java
 44562537bc426e47a42d680bb410dc59bf9bd854 
  
geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
 a4041e198f1a9a4961915504c51256d0b03aa7c2 
  
geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceFactory.java
 02f34f15617f7bf4ad9ee7fa51f32be4db3c198a 
  
geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceType.java
 8ae76d22b628b3175db45116b80dfcfbe34aba1d 
  
geode-core/src/main/java/org/apache/geode/internal/security/shiro/ConfigInitializer.java
 60f014b9c33732a4ea134a654e666a9439b210bb 
  
geode-core/src/main/java/org/apache/geode/internal/security/shiro/CustomAuthRealm.java
 51449fdd5570494f3cf91325985a5eb9fc9f6d57 
  
geode-core/src/main/java/org/apache/geode/internal/security/shiro/RealmInitializer.java
 978c4dd0ab92afde53972f7feb9d8f018d0bf662 
  
geode-core/src/main/java/org/apache/geode/internal/security/shiro/ShiroSecurityManagerInitializer.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/membership/MembershipJUnitTest.java
 a0c3cf3074051990cc50755131f8024db0b1faad 
  
geode-core/src/test/java/org/apache/geode/internal/security/DisabledSecurityServiceTest.java
 cacbeed957c3b87d08c93db74e38e0134565f699 
  
geode-core/src/test/java/org/apache/geode/internal/security/EnabledSecurityServiceTest.java
 fca7eae9413cee98d351db5349fd950d3aa56180 
  
geode-core/src/test/java/org/apache/geode/internal/security/FakePostProcessor.java
 70823443b5c4f776c86bb28ed49a73e690c5c872 
  
geode-core/src/test/java/org/apache/geode/internal/security/FakeSecurityManager.java
 ca4e6b7fb462bb4e3fefbc5db8a9503c6b13a865 
  
geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceConstructorTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/internal/security/LegacySecurityServiceTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryShiroIntegrationTest.java
 89070123978c22c4cfa8684fbb5b033dc9d83ffa 
  
geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryTest.java
 f027a4367b38681f83dad2d4c4add67759633644 
  
geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceTest.java
 44893520962331bcd41e972afa661538c28d4fb2 
  
geode-core/src/test/java/org/apache/geode/internal/security/shiro/ConfigInitializerIntegrationTest.java
 857c0be8940b4acde2aa4992fac0122b687391c2 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithCustomRealmIntegrationTest.java
 01d6bb6488e76fb3cf652ad211e9f7e2fac51389 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithShiroIniIntegrationTest.java
 1caedbcede239d6a96640381cc6941948637b442 
  
geode-core/src/test/java/org/apache/geode/security/CacheFactoryWithSecurityObjectTest.java
 cdb90f1b580edaf6a2762883d4159a45d69c4728 
  
geode-core/src/test/java/org/apache/geode/security/SecurityManagerLifecycleDistributedTest.java
 a9048b9219a494f61e3873ee3f2908da04bf6154 
  geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/Server.java 
63f907cb007846626a9a66dc6b1ef28e0bb6db45 
  
geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java
 767588d3717fccbb0b9c7dec7c5439e16d5381aa 


Diff: https://reviews.apache.org/r/60028/diff/2/

Changes: 

Re: Review Request 60028: GEODE-2632: consolidate different types of SecurityService

[Jinmei Liao]

> On June 13, 2017, 7 p.m., Kirk Lund wrote:
> > -1 I don't see why these changes are needed other than to wipe out what I 
> > did? For example, you've undone my introduction of RealmInitializer and 
> > ConfigInitializer. I introduced those so I can continue writing more 
> > mocking tests for SecurityService so that our unit tests don't have to be 
> > integration tests that actually interact with Shiro. We should be able to 
> > separate our code. Also, I really prefer using a "DisabledSecurityService" 
> > when security is disabled rather than using a "LegacySecurityService" -- 
> > when security is disabled, it isn't "legacy".

what about the consolidation between the EnabledSecurityService and 
CustomSecurityService? There are lots of duplicated code there. Also regarding 
DisabledSecurityService, as far the product is concerned, there is no 
DisabledSecurityService though, only test would use them, so I think it would 
be a good idea to lump them together. It does get rid of a log of duplicate 
code though.

I do agree that the RealmInitializer would help for mocking. I will bring it 
back.


- Jinmei


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


On June 13, 2017, 4:49 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60028/
> ---
> 
> (Updated June 13, 2017, 4:49 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * combine EnabledSecurityService and CustomSecurityService into 
> IntegratedSecurityService
> * combine DisabledSecurityService and LegacySecurityService
> * provide default impelementations of SecurityService
> * consolidate SecurityService creation.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
>  22edb6f06c7791929cc9a033ca1a1bfed5751a47 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/CallbackInstantiator.java
>  3ff632d3857189513243959ee96da89da66d5a27 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/CustomSecurityService.java
>  0ba1cb62468f15fda60a94dac9c781fe1793b28f 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/DisabledSecurityService.java
>  b50569036db1acac927f08ee5c1771c72ede3c1d 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/EnabledSecurityService.java
>  f0568c0ebc2b17c47b5059c60aa4f8bd500c5d6c 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/LegacySecurityService.java
>  44562537bc426e47a42d680bb410dc59bf9bd854 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
>  a4041e198f1a9a4961915504c51256d0b03aa7c2 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceFactory.java
>  02f34f15617f7bf4ad9ee7fa51f32be4db3c198a 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceType.java
>  8ae76d22b628b3175db45116b80dfcfbe34aba1d 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/shiro/ConfigInitializer.java
>  60f014b9c33732a4ea134a654e666a9439b210bb 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/shiro/CustomAuthRealm.java
>  51449fdd5570494f3cf91325985a5eb9fc9f6d57 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/shiro/RealmInitializer.java
>  978c4dd0ab92afde53972f7feb9d8f018d0bf662 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/membership/MembershipJUnitTest.java
>  a0c3cf3074051990cc50755131f8024db0b1faad 
>   
> geode-core/src/test/java/org/apache/geode/internal/security/DisabledSecurityServiceTest.java
>  cacbeed957c3b87d08c93db74e38e0134565f699 
>   
> geode-core/src/test/java/org/apache/geode/internal/security/EnabledSecurityServiceTest.java
>  fca7eae9413cee98d351db5349fd950d3aa56180 
>   
> geode-core/src/test/java/org/apache/geode/internal/security/FakePostProcessor.java
>  70823443b5c4f776c86bb28ed49a73e690c5c872 
>   
> geode-core/src/test/java/org/apache/geode/internal/security/FakeSecurityManager.java
>  ca4e6b7fb462bb4e3fefbc5db8a9503c6b13a865 
>   
> geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceConstructorTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/security/LegacySecurityServiceTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryShiroIntegrationTest.java
>  89070123978c22c4cfa8684fbb5b033dc9d83ffa 
>   
> 

[GitHub] geode pull request #577: Feature/geode 3049

[dschneider-pivotal]

Github user dschneider-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/577#discussion_r121776319
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionRedundancyTracker.java
 ---
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for 
additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF 
ANY KIND, either express
+ * or implied. See the License for the specific language governing 
permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.internal.cache;
+
+import org.apache.geode.internal.i18n.LocalizedStrings;
+import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.internal.logging.log4j.LocalizedMessage;
+import org.apache.logging.log4j.Logger;
+
+/**
+ * Keeps track redundancy statistics across the buckets of a given {@link 
PartitionedRegion}
+ */
+public class PartitionedRegionRedundancyTracker {
+  private static final Logger logger = LogService.getLogger();
+
+  private final PartitionedRegionStats stats;
+  private final String regionPath;
+  private final int totalBuckets;
+  private final int targetRedundancy;
+
+  private int lowRedundancyBuckets;
+  private int noCopiesBuckets;
+  private int lowestBucketCopies;
+
+  /**
+   * Creates a new PartitionedRegionRedundancyTracker
+   *
+   * @param totalBuckets number of buckets in the region to track
+   * @param redundantCopies number of redundant copies specified on the 
region to track
+   * @param stats the statistics container for the region to track
+   * @param regionPath the full path of the region to track
+   */
+  PartitionedRegionRedundancyTracker(int totalBuckets, int redundantCopies,
+  PartitionedRegionStats stats, String regionPath) {
+this.stats = stats;
+this.regionPath = regionPath;
+this.totalBuckets = totalBuckets;
+this.targetRedundancy = redundantCopies;
+this.lowestBucketCopies = redundantCopies + 1;
+  }
+
+  /**
+   * Since consistency was last reached, provides the lowest number of 
copies of a bucket that have
+   * been remaining across all the buckets in the region
+   *
+   * @return the number of copies of the bucket with the least copies 
available
+   */
+  int getLowestBucketCopies() {
+return lowestBucketCopies;
+  }
+
+  /**
+   * Increments the count of buckets that do not meet redundancy
+   */
+  synchronized void incrementLowRedundancyBucketCount() {
+if (lowRedundancyBuckets == totalBuckets) {
+  return;
+}
+lowRedundancyBuckets++;
+stats.incLowRedundancyBucketCount(1);
+  }
+
+  /**
+   * Updates the count of copies for the bucket with the least copies if a 
new low has been reached
+   * 
+   * @param bucketCopies number of copies of a bucket remaining
+   */
+  synchronized void reportBucketCount(int bucketCopies) {
+if (bucketCopies < lowestBucketCopies) {
+  lowestBucketCopies = bucketCopies;
+  logger.warn(LocalizedMessage.create(
+  
LocalizedStrings.BucketAdvisor_REDUNDANCY_HAS_DROPPED_BELOW_0_CONFIGURED_COPIES_TO_1_ACTUAL_COPIES_FOR_2,
+  new Object[] {targetRedundancy, bucketCopies + 1, regionPath}));
+}
+  }
+
+  /**
+   * Increments the count of buckets that no longer have any copies 
remaining
+   */
+  synchronized void incrementNoCopiesBucketCount() {
+if (noCopiesBuckets == totalBuckets) {
+  return;
+}
+noCopiesBuckets++;
+stats.incNoCopiesBucketCount(1);
+if (noCopiesBuckets == 1) {
+  logger.warn("No copies remain for " + regionPath);
--- End diff --

I think this warning needs to say more. We should let them know that it may 
be ok but that data may have been lost


---
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.
---

[GitHub] geode pull request #577: Feature/geode 3049

[dschneider-pivotal]

Github user dschneider-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/577#discussion_r121751707
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/cache/BucketRedundancyTracker.java
 ---
@@ -0,0 +1,108 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for 
additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF 
ANY KIND, either express
+ * or implied. See the License for the specific language governing 
permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.internal.cache;
+
+/**
+ * Keeps track of redundancy status for a PartitionedRegion and (if 
enabled) updates the statistics
+ * for the region.
+ */
+class BucketRedundancyTracker {
+  private boolean redundancySatisfied = false;
--- End diff --

It is not clear, from this class, what makes access to these non final non 
volatile instance variables safe.
Should you add comments on the methods that modify them that those methods 
are not thread safe and the caller is responsible for making sure they are not 
concurrently called by multiple threads?


---
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.
---

[GitHub] geode pull request #577: Feature/geode 3049

[dschneider-pivotal]

Github user dschneider-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/577#discussion_r121742105
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/cache/BucketRedundancyTracker.java
 ---
@@ -0,0 +1,108 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for 
additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF 
ANY KIND, either express
+ * or implied. See the License for the specific language governing 
permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.internal.cache;
+
+/**
+ * Keeps track of redundancy status for a PartitionedRegion and (if 
enabled) updates the statistics
--- End diff --

did you mean to say BucketRegion instead of PartitionedRegion?


---
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.
---

Re: Review Request 60030: GEODE-2925: finer security for disk management

[Patrick Rhomberg]

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




geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java
Lines 200-209 (patched)


Kirk has the idea for

```
if (StringUtils.isNotBlank(regionFunctionArgs.getDiskStore())) {
getSecurityService().authorize(Resource.CLUSTER, Operation.WRITE, 
Target.DISK);
}
```

That doesn't read quite as clearly as what you have, but can a persistent 
region only be instantiated via these region shortcuts?  Or could a 
customization get around this check?  (This is a case of me still not knowing 
the use cases of the product.)



geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java
Lines 208 (patched)


I realize this from a commit from yesterday, but is there a reason we went 
with `authorizeDiskManage` instead of `authorizeClusterManageDisk` and the 
like?  I was confused for a second and thought that you had duplicated the same 
auth that the `@ResourceOperation` specified.


- Patrick Rhomberg


On June 12, 2017, 9:14 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60030/
> ---
> 
> (Updated June 12, 2017, 9:14 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2925: finer security for disk management
> 
> changes that are needed to apply the finer grained security for disk 
> management after the latest security changes.
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/management/DiskStoreMXBean.java 
> cca6272610517d75c877945872e1b9e77efae230 
>   
> geode-core/src/main/java/org/apache/geode/management/DistributedSystemMXBean.java
>  f6f701e1faad30839574d8b20fbba31032755e80 
>   geode-core/src/main/java/org/apache/geode/management/MemberMXBean.java 
> e2de400601075af880a04b4521898280249d3e99 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemMBean.java
>  c45da73b4aabfc389b31f9a1092f2abd883813f7 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java
>  b2f1d561f9ef30dd2c02680b0a1075296719cd61 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
>  ef2c3dd5db5f193de1ef307813b02f2ef60a4715 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/QueueCommands.java
>  0f407e7d3273f4ec6206e24bbc064e207335ee11 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/MBeanServerWrapper.java
>  47260bc855025864d0def5855eff33d0cdfd6617 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/DiskStoreMXBeanSecurityJUnitTest.java
>  e6502c0e4e94b2cea09ff78ea39ebcf02a7197f7 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/DistributedSystemMXBeanSecurityTest.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60030/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 60010: GEODE-3062: replace SecurityService after loading cluster config

[Patrick Rhomberg]

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




geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigWithSecurityDUnitTest.java
Line 67 (original), 68 (patched)


You're doing the lords' work here.


- Patrick Rhomberg


On June 13, 2017, 4:29 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60010/
> ---
> 
> (Updated June 13, 2017, 4:29 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jinmei Liao, Jared Stewart, Ken Howe, 
> and Patrick Rhomberg.
> 
> 
> Bugs: GEODE-3062
> https://issues.apache.org/jira/browse/GEODE-3062
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Add new test to ClusterConfigWithSecurityDUnitTest that fails due to bug 
> GEODE-3062.
> 
> Remove unused Cache param from applyClusterPropertiesConfiguration so it can 
> be called during Cache construction.
> 
> Move cluster config request to Cache construction and handle jars and 
> properties there. Create new SecurityService in constructor and overwrite the 
> SecurityService in InternalDistributedSystem.
> 
> NOTE: We will later have to fix GEODE-3061 by moving cluster config request 
> from Cache to InternalDistributedSystem construction so that IDS can be 
> created with gemfire.properties from cluster config. At that time we would 
> rip out both cluster config request and creation of security service from 
> Cache construction and pass both into Cache construction.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
>  22edb6f06c7791929cc9a033ca1a1bfed5751a47 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java
>  4f4881fe39116faa505bc2fbec74efd669efe0ef 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
>  40df0c7dcac8827a381c268c1f90e6acfb97a7f1 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigWithSecurityDUnitTest.java
>  c551ca9104a85dcf54c0bebbc4178fce4114a416 
> 
> 
> Diff: https://reviews.apache.org/r/60010/diff/2/
> 
> 
> Testing
> ---
> 
> Precheckin passes
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Re: Review Request 60010: GEODE-3062: replace SecurityService after loading cluster config

[Patrick Rhomberg]

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


Ship it!




Ship It!

- Patrick Rhomberg


On June 13, 2017, 4:29 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60010/
> ---
> 
> (Updated June 13, 2017, 4:29 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jinmei Liao, Jared Stewart, Ken Howe, 
> and Patrick Rhomberg.
> 
> 
> Bugs: GEODE-3062
> https://issues.apache.org/jira/browse/GEODE-3062
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Add new test to ClusterConfigWithSecurityDUnitTest that fails due to bug 
> GEODE-3062.
> 
> Remove unused Cache param from applyClusterPropertiesConfiguration so it can 
> be called during Cache construction.
> 
> Move cluster config request to Cache construction and handle jars and 
> properties there. Create new SecurityService in constructor and overwrite the 
> SecurityService in InternalDistributedSystem.
> 
> NOTE: We will later have to fix GEODE-3061 by moving cluster config request 
> from Cache to InternalDistributedSystem construction so that IDS can be 
> created with gemfire.properties from cluster config. At that time we would 
> rip out both cluster config request and creation of security service from 
> Cache construction and pass both into Cache construction.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
>  22edb6f06c7791929cc9a033ca1a1bfed5751a47 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java
>  4f4881fe39116faa505bc2fbec74efd669efe0ef 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
>  40df0c7dcac8827a381c268c1f90e6acfb97a7f1 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigWithSecurityDUnitTest.java
>  c551ca9104a85dcf54c0bebbc4178fce4114a416 
> 
> 
> Diff: https://reviews.apache.org/r/60010/diff/2/
> 
> 
> Testing
> ---
> 
> Precheckin passes
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Re: Review Request 60028: GEODE-2632: consolidate different types of SecurityService

[Kirk Lund]

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



-1 I don't see why these changes are needed other than to wipe out what I did? 
For example, you've undone my introduction of RealmInitializer and 
ConfigInitializer. I introduced those so I can continue writing more mocking 
tests for SecurityService so that our unit tests don't have to be integration 
tests that actually interact with Shiro. We should be able to separate our 
code. Also, I really prefer using a "DisabledSecurityService" when security is 
disabled rather than using a "LegacySecurityService" -- when security is 
disabled, it isn't "legacy".

- Kirk Lund


On June 13, 2017, 4:49 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60028/
> ---
> 
> (Updated June 13, 2017, 4:49 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * combine EnabledSecurityService and CustomSecurityService into 
> IntegratedSecurityService
> * combine DisabledSecurityService and LegacySecurityService
> * provide default impelementations of SecurityService
> * consolidate SecurityService creation.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
>  22edb6f06c7791929cc9a033ca1a1bfed5751a47 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/CallbackInstantiator.java
>  3ff632d3857189513243959ee96da89da66d5a27 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/CustomSecurityService.java
>  0ba1cb62468f15fda60a94dac9c781fe1793b28f 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/DisabledSecurityService.java
>  b50569036db1acac927f08ee5c1771c72ede3c1d 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/EnabledSecurityService.java
>  f0568c0ebc2b17c47b5059c60aa4f8bd500c5d6c 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/LegacySecurityService.java
>  44562537bc426e47a42d680bb410dc59bf9bd854 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
>  a4041e198f1a9a4961915504c51256d0b03aa7c2 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceFactory.java
>  02f34f15617f7bf4ad9ee7fa51f32be4db3c198a 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceType.java
>  8ae76d22b628b3175db45116b80dfcfbe34aba1d 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/shiro/ConfigInitializer.java
>  60f014b9c33732a4ea134a654e666a9439b210bb 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/shiro/CustomAuthRealm.java
>  51449fdd5570494f3cf91325985a5eb9fc9f6d57 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/shiro/RealmInitializer.java
>  978c4dd0ab92afde53972f7feb9d8f018d0bf662 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/membership/MembershipJUnitTest.java
>  a0c3cf3074051990cc50755131f8024db0b1faad 
>   
> geode-core/src/test/java/org/apache/geode/internal/security/DisabledSecurityServiceTest.java
>  cacbeed957c3b87d08c93db74e38e0134565f699 
>   
> geode-core/src/test/java/org/apache/geode/internal/security/EnabledSecurityServiceTest.java
>  fca7eae9413cee98d351db5349fd950d3aa56180 
>   
> geode-core/src/test/java/org/apache/geode/internal/security/FakePostProcessor.java
>  70823443b5c4f776c86bb28ed49a73e690c5c872 
>   
> geode-core/src/test/java/org/apache/geode/internal/security/FakeSecurityManager.java
>  ca4e6b7fb462bb4e3fefbc5db8a9503c6b13a865 
>   
> geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceConstructorTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/security/LegacySecurityServiceTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryShiroIntegrationTest.java
>  89070123978c22c4cfa8684fbb5b033dc9d83ffa 
>   
> geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryTest.java
>  f027a4367b38681f83dad2d4c4add67759633644 
>   
> geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceTest.java
>  44893520962331bcd41e972afa661538c28d4fb2 
>   
> geode-core/src/test/java/org/apache/geode/internal/security/shiro/ConfigInitializerIntegrationTest.java
>  857c0be8940b4acde2aa4992fac0122b687391c2 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigWithSecurityDUnitTest.java
>  c551ca9104a85dcf54c0bebbc4178fce4114a416 
>   
> 

Re: Review Request 60010: GEODE-3062: replace SecurityService after loading cluster config

[Emily Yeh]

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


Ship it!




Ship It!

- Emily Yeh


On June 13, 2017, 4:29 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60010/
> ---
> 
> (Updated June 13, 2017, 4:29 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jinmei Liao, Jared Stewart, Ken Howe, 
> and Patrick Rhomberg.
> 
> 
> Bugs: GEODE-3062
> https://issues.apache.org/jira/browse/GEODE-3062
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Add new test to ClusterConfigWithSecurityDUnitTest that fails due to bug 
> GEODE-3062.
> 
> Remove unused Cache param from applyClusterPropertiesConfiguration so it can 
> be called during Cache construction.
> 
> Move cluster config request to Cache construction and handle jars and 
> properties there. Create new SecurityService in constructor and overwrite the 
> SecurityService in InternalDistributedSystem.
> 
> NOTE: We will later have to fix GEODE-3061 by moving cluster config request 
> from Cache to InternalDistributedSystem construction so that IDS can be 
> created with gemfire.properties from cluster config. At that time we would 
> rip out both cluster config request and creation of security service from 
> Cache construction and pass both into Cache construction.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
>  22edb6f06c7791929cc9a033ca1a1bfed5751a47 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java
>  4f4881fe39116faa505bc2fbec74efd669efe0ef 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
>  40df0c7dcac8827a381c268c1f90e6acfb97a7f1 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigWithSecurityDUnitTest.java
>  c551ca9104a85dcf54c0bebbc4178fce4114a416 
> 
> 
> Diff: https://reviews.apache.org/r/60010/diff/2/
> 
> 
> Testing
> ---
> 
> Precheckin passes
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Build failed in Jenkins: Geode-release #62

[Apache Jenkins Server]

See 


Changes:

[dbarnes] Update geode-book README instructions

--
[...truncated 119.41 KB...]
:geode-old-client-support:jar
:geode-old-client-support:javadoc
:geode-old-client-support:javadocJar
:geode-old-client-support:sourcesJar
:geode-old-client-support:signArchives SKIPPED
:geode-pulse:javadoc
:geode-pulse:javadocJar
:geode-pulse:sourcesJar
:geode-pulse:war
:geode-pulse:signArchives SKIPPED
:geode-rebalancer:jar
:geode-rebalancer:javadoc
:geode-rebalancer:javadocJar
:geode-rebalancer:sourcesJar
:geode-rebalancer:signArchives SKIPPED
:geode-wan:jar
:geode-wan:javadoc
:geode-wan:javadocJar
:geode-wan:sourcesJar
:geode-wan:signArchives SKIPPED
:geode-web:javadoc UP-TO-DATE
:geode-web:javadocJar
:geode-web:sourcesJar
:geode-web:war
:geode-web:signArchives SKIPPED
:geode-web-api:javadoc
:geode-web-api:javadocJar
:geode-web-api:sourcesJar
:geode-web-api:war
:geode-web-api:signArchives SKIPPED
:geode-assembly:distTar
:geode-assembly:distZip
:geode-assembly:writeBuildInfo
:geode-assembly:srcDistTar
:geode-assembly:srcDistZip
:geode-assembly:signArchives SKIPPED
:geode-assembly:assemble
:geode-pulse:jar
:geode-assembly: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-assembly:processTestResources
:geode-assembly:testClasses
:geode-assembly:checkMissedTests
:geode-assembly:spotlessJavaCheck
:geode-assembly:spotlessCheck
:geode-assembly:installDist
:geode-assembly:test
:geode-assembly:check
:geode-assembly:build
:geode-assembly:distributedTest
:geode-assembly:integrationTest
:geode-benchmarks:jar
:geode-benchmarks:javadoc UP-TO-DATE
:geode-benchmarks:javadocJar
:geode-benchmarks:sourcesJar
:geode-benchmarks:signArchives SKIPPED
:geode-benchmarks:assemble
:geode-benchmarks:compileTestJava UP-TO-DATE
:geode-benchmarks:processTestResources UP-TO-DATE
:geode-benchmarks:testClasses UP-TO-DATE
:geode-benchmarks:checkMissedTests UP-TO-DATE
:geode-benchmarks:spotlessJavaCheck
:geode-benchmarks:spotlessCheck
:geode-benchmarks:test UP-TO-DATE
:geode-benchmarks:check
:geode-benchmarks:build
:geode-benchmarks:distributedTest UP-TO-DATE
:geode-benchmarks:integrationTest UP-TO-DATE
:geode-common:assemble
:geode-common:compileTestJava
:geode-common:processTestResources UP-TO-DATE
:geode-common:testClasses
:geode-common:checkMissedTests
:geode-common:spotlessJavaCheck
:geode-common:spotlessCheck
:geode-common:test
:geode-common:check
:geode-common:build
:geode-common:distributedTest
:geode-common:integrationTest
:geode-core:assemble
:geode-core:checkMissedTests
:geode-core:spotlessJavaCheck
:geode-core:spotlessCheck
:geode-core:test
:geode-core:check
:geode-core:build
:geode-core:distributedTest
: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:integrationTest
:geode-json:assemble
:geode-json:compileTestJava UP-TO-DATE
:geode-json:processTestResources
:geode-json:testClasses
: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: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:integrationTest
:geode-lucene:assemble
:geode-lucene:compileTestJava
Download 
https://repo1.maven.org/maven2/org/apache/lucene/lucene-test-framework/6.4.1/lucene-test-framework-6.4.1.pom
Download 
https://repo1.maven.org/maven2/org/apache/lucene/lucene-codecs/6.4.1/lucene-codecs-6.4.1.pom
Download 
https://repo1.maven.org/maven2/org/apache/lucene/lucene-analyzers-phonetic/6.4.1/lucene-analyzers-phonetic-6.4.1.pom
Download 
https://repo1.maven.org/maven2/com/carrotsearch/randomizedtesting/randomizedtesting-runner/2.4.0/randomizedtesting-runner-2.4.0.pom
Download 
https://repo1.maven.org/maven2/com/carrotsearch/randomizedtesting/randomizedtesting-parent/2.4.0/randomizedtesting-parent-2.4.0.pom
Download 

Re: Review Request 60052: GEODE-2294: revert to avoid rest protocol change

[Emily Yeh]

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


Ship it!




Ship It!

- Emily Yeh


On June 13, 2017, 5:29 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60052/
> ---
> 
> (Updated June 13, 2017, 5:29 p.m.)
> 
> 
> Review request for geode, Anthony Baker, Emily Yeh, Jared Stewart, Ken Howe, 
> Kirk Lund, and Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2294: revert to avoid rest protocol change
> 
> 
> Diffs
> -
> 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/AbstractBaseController.java
>  d8eb5721315b65f2924a54e565a1dad2815a5afa 
>   
> geode-web-api/src/test/java/org/apache/geode/rest/internal/web/controllers/AbstractBaseControllerJUnitTest.java
>  7ebfbcfe9d97a2e984b2b9975b95945971d4b1ef 
> 
> 
> Diff: https://reviews.apache.org/r/60052/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 59811: GEODE-2420: add file-size-limit param to the ExportLogsController

[Emily Yeh]

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


Ship it!




Ship It!

- Emily Yeh


On June 8, 2017, 9:08 p.m., Ken Howe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59811/
> ---
> 
> (Updated June 8, 2017, 9:08 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jinmei Liao, Jared Stewart, Kirk Lund, 
> and Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2420: add file-size-limit param to the ExportLogsController
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
>  0ff780cbf66937d8ececfb3a2d0789ee485b9b62 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java
>  57355c0efae4c6da9470267f95e27e59aa4d8b2c 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
>  9f68d3a5eaadbe8f1bd95ec8df85ed1f65aa67ce 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ExportLogController.java
>  a369c6e1ffb330715fbde2cd69d023ed36f133ad 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java
>  16549e70bbebf4390bb73a481274e92ca6cad035 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java
>  8609b3aaf0a0eb1ba903bd39c64103f9510a6a78 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java
>  09ee08dd6af29b9a418ef7499defc4980da787ed 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsStatsDUnitTest.java
>  44a036298e0991c880fc552596d296e104b97ca1 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java
>  4e1dac013d239437829bc52dc70689c4ba15dc58 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionTest.java
>  cc5e7d5256741ad0a48ff87c7f989a18b90f7f03 
> 
> 
> Diff: https://reviews.apache.org/r/59811/diff/4/
> 
> 
> Testing
> ---
> 
> 6/7/17: re-started precheckin
> precheckin is green with the exception of unrelated DUnit tests
> 
> * re-starting precheckin once more
> 
> 6/8/17: previous precheckin green except for 1 known flaky test
> 
> Precheckin started
> 
> 
> Thanks,
> 
> Ken Howe
> 
>

[GitHub] geode issue #573: GEODE-2622: Fix test failures caused by upgrade to Mockito...

[srikanthmanvi]

Github user srikanthmanvi commented on the issue:

https://github.com/apache/geode/pull/573
  
Thank you Kirk, I will close this PR.


---
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.
---

[GitHub] geode pull request #573: GEODE-2622: Fix test failures caused by upgrade to ...

[srikanthmanvi]

Github user srikanthmanvi closed the pull request at:

https://github.com/apache/geode/pull/573


---
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.
---

Re: Review Request 60052: GEODE-2294: revert to avoid rest protocol change

[Anthony Baker]

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


Ship it!




Ship It!

- Anthony Baker


On June 13, 2017, 5:29 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60052/
> ---
> 
> (Updated June 13, 2017, 5:29 p.m.)
> 
> 
> Review request for geode, Anthony Baker, Emily Yeh, Jared Stewart, Ken Howe, 
> Kirk Lund, and Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2294: revert to avoid rest protocol change
> 
> 
> Diffs
> -
> 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/AbstractBaseController.java
>  d8eb5721315b65f2924a54e565a1dad2815a5afa 
>   
> geode-web-api/src/test/java/org/apache/geode/rest/internal/web/controllers/AbstractBaseControllerJUnitTest.java
>  7ebfbcfe9d97a2e984b2b9975b95945971d4b1ef 
> 
> 
> Diff: https://reviews.apache.org/r/60052/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

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

[Kirk Lund]

Thanks Mark!

On Tue, Jun 13, 2017 at 10:11 AM, Mark Bretl  wrote:

> Done
>
> --Mark
>
> On Tue, Jun 13, 2017 at 9:58 AM, Kirk Lund  wrote:
>
> > Can someone with Jenkins permissions please excluded mesos2 from being
> used
> > for our nightly builds?
> >
> > ERROR: JAVA_HOME is set to an invalid directory:
> /home/jenkins/tools/java/
> > latest1.8
> >
> > -- Forwarded message --
> > From: Apache Jenkins Server 
> > Date: Mon, Jun 12, 2017 at 8:42 PM
> > Subject: Build failed in Jenkins: Geode-nightly #865
> > To: dev@geode.apache.org, kmil...@pivotal.io, jstew...@pivotal.io,
> > dbar...@pivotal.io, kl...@pivotal.io, kh...@apache.org, n...@pivotal.io,
> > bschucha...@pivotal.io
> >
> >
> > See  > redirect?page=changes>
> >
> > Changes:
> >
> > [jiliao] GEODE-2925: add target for resource operation for finer grained
> > security
> >
> > [dbarnes] Update geode-book README instructions
> >
> > [klund] GEODE-290: Remove deprecated methods from launcher classes
> >
> > [jiliao] GEODE-2928: get rid of the isGfshVM static variable
> >
> > [jiliao] GEODE-2928: get rid of the isGfshVM static variable - fix merge
> > issues
> >
> > [nnag] GEODE-3066: Test tweaks to stabilize it.
> >
> > [jstewart] GEODE-3048: Introduce a rule to identify tests that require
> > GEODE_HOME
> >
> > [klund] GEODE-3068: fix alphabetic ordering
> >
> > --
> > Started by timer
> > [EnvInject] - Loading node environment variables.
> > Building remotely on mesos2 (mesos s390x) in workspace <
> > https://builds.apache.org/job/Geode-nightly/ws/>
> >  > git rev-parse --is-inside-work-tree # timeout=10
> > Fetching changes from the remote Git repository
> >  > git config remote.origin.url https://git-wip-us.apache.org/
> > repos/asf/geode.git # timeout=10
> > Fetching upstream changes from https://git-wip-us.apache.org/
> > repos/asf/geode.git
> >  > git --version # timeout=10
> >  > git fetch --tags --progress https://git-wip-us.apache.org/
> > repos/asf/geode.git +refs/heads/*:refs/remotes/origin/*
> >  > git rev-parse refs/remotes/origin/develop^{commit} # timeout=10
> >  > git rev-parse refs/remotes/origin/origin/develop^{commit} #
> timeout=10
> > Checking out Revision 65471c1174df161c46237f87145d81d27a1663a9
> > (refs/remotes/origin/develop)
> >  > git config core.sparsecheckout # timeout=10
> >  > git checkout -f 65471c1174df161c46237f87145d81d27a1663a9
> >  > git branch -a -v --no-abbrev # timeout=10
> >  > git branch -D develop # timeout=10
> >  > git checkout -b develop 65471c1174df161c46237f87145d81d27a1663a9
> >  > git rev-list 122b07afde7cd56898fae354fadc78fdd5dc721d # timeout=10
> > [Geode-nightly] $ /bin/bash -xe /tmp/hudson4018735290081826255.sh
> > + git clean -d -f gemfire-pulse/src/main/resources/
> > [Gradle] - Launching build.
> > [Geode-nightly] $  job/Geode-nightly/ws/gradlew>
> > --no-daemon clean precheckin distTar distZip uploadArchives -xflakyTest
> >
> > ERROR: JAVA_HOME is set to an invalid directory:
> /home/jenkins/tools/java/
> > latest1.8
> >
> > Please set the JAVA_HOME variable in your environment to match the
> > location of your Java installation.
> >
> > Build step 'Invoke Gradle script' changed build result to FAILURE
> > Build step 'Invoke Gradle script' marked build as failure
> > Archiving artifacts
> > Recording test results
> > ERROR: Step ?Publish JUnit test result report? failed: No test report
> files
> > were found. Configuration error?
> > Not sending mail to unregistered user ukohlme...@pivotal.io
> > Not sending mail to unregistered user huyn...@gmail.com
> > Not sending mail to unregistered user jil...@pivotal.io
> >
>

Review Request 60052: GEODE-2294: revert to avoid rest protocol change

[Jinmei Liao]

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

Review request for geode, Anthony Baker, Emily Yeh, Jared Stewart, Ken Howe, 
Kirk Lund, and Patrick Rhomberg.


Repository: geode


Description
---

GEODE-2294: revert to avoid rest protocol change


Diffs
-

  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/AbstractBaseController.java
 d8eb5721315b65f2924a54e565a1dad2815a5afa 
  
geode-web-api/src/test/java/org/apache/geode/rest/internal/web/controllers/AbstractBaseControllerJUnitTest.java
 7ebfbcfe9d97a2e984b2b9975b95945971d4b1ef 


Diff: https://reviews.apache.org/r/60052/diff/1/


Testing
---

precheckin running


Thanks,

Jinmei Liao

[GitHub] geode issue #578: GEODE-1958: Removing PasswordUtil

[YehEmily]

Github user YehEmily commented on the issue:

https://github.com/apache/geode/pull/578
  
Precheckin is mostly green, with one (seemingly unrelated) test failure! 
I'm rerunning the test and looking into the failure now.


---
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.
---

[GitHub] geode pull request #578: GEODE-1958: Removing PasswordUtil

[YehEmily]

GitHub user YehEmily opened a pull request:

https://github.com/apache/geode/pull/578

GEODE-1958: Removing PasswordUtil

[View the original JIRA ticket 
here.](https://issues.apache.org/jira/browse/GEODE-1958)

`PasswordUtil.java` contained methods used to encrypt a password to be 
stored in `cache.xml`, which was an unsafe way to handle security. As a result, 
most methods in `PasswordUtil.java` were removed _except_ `decrypt(String 
password)`, since we want to maintain backwards compatibility. `decrypt(String 
password)` has been deprecated, and references to encrypting passwords have 
been removed.

Thank you for submitting a contribution to Apache Geode.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in 
the commit message?

- [ ] Has your PR been rebased against the latest commit within the target 
branch (typically `develop`)?

- [ ] Is your initial contribution a single, squashed commit?

- [ ] Does `gradlew build` run cleanly?

- [ ] Have you written or updated unit tests to verify your changes?

- [ ] If adding new dependencies to the code, are these dependencies 
licensed in a way that is compatible for inclusion under [ASF 
2.0](http://www.apache.org/legal/resolved.html#category-a)?

### Note:
Please ensure that once the PR is submitted, you check travis-ci for build 
issues and
submit an update to your PR as soon as possible. If you need help, please 
send an
email to dev@geode.apache.org.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/YehEmily/geode GEODE-1958-v3

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/geode/pull/578.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 #578


commit 2f08e08fa7fec127324ee47a338d331bf059bbf6
Author: YehEmily 
Date:   2017-06-12T18:42:15Z

GEODE-1958: Working on removing PasswordUtil and all related commands, 
classes, etc. Keeping decrypt() method to maintain backwards compatibility.




---
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.
---

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

[Mark Bretl]

Done

--Mark

On Tue, Jun 13, 2017 at 9:58 AM, Kirk Lund  wrote:

> Can someone with Jenkins permissions please excluded mesos2 from being used
> for our nightly builds?
>
> ERROR: JAVA_HOME is set to an invalid directory: /home/jenkins/tools/java/
> latest1.8
>
> -- Forwarded message --
> From: Apache Jenkins Server 
> Date: Mon, Jun 12, 2017 at 8:42 PM
> Subject: Build failed in Jenkins: Geode-nightly #865
> To: dev@geode.apache.org, kmil...@pivotal.io, jstew...@pivotal.io,
> dbar...@pivotal.io, kl...@pivotal.io, kh...@apache.org, n...@pivotal.io,
> bschucha...@pivotal.io
>
>
> See  redirect?page=changes>
>
> Changes:
>
> [jiliao] GEODE-2925: add target for resource operation for finer grained
> security
>
> [dbarnes] Update geode-book README instructions
>
> [klund] GEODE-290: Remove deprecated methods from launcher classes
>
> [jiliao] GEODE-2928: get rid of the isGfshVM static variable
>
> [jiliao] GEODE-2928: get rid of the isGfshVM static variable - fix merge
> issues
>
> [nnag] GEODE-3066: Test tweaks to stabilize it.
>
> [jstewart] GEODE-3048: Introduce a rule to identify tests that require
> GEODE_HOME
>
> [klund] GEODE-3068: fix alphabetic ordering
>
> --
> Started by timer
> [EnvInject] - Loading node environment variables.
> Building remotely on mesos2 (mesos s390x) in workspace <
> https://builds.apache.org/job/Geode-nightly/ws/>
>  > git rev-parse --is-inside-work-tree # timeout=10
> Fetching changes from the remote Git repository
>  > git config remote.origin.url https://git-wip-us.apache.org/
> repos/asf/geode.git # timeout=10
> Fetching upstream changes from https://git-wip-us.apache.org/
> repos/asf/geode.git
>  > git --version # timeout=10
>  > git fetch --tags --progress https://git-wip-us.apache.org/
> repos/asf/geode.git +refs/heads/*:refs/remotes/origin/*
>  > git rev-parse refs/remotes/origin/develop^{commit} # timeout=10
>  > git rev-parse refs/remotes/origin/origin/develop^{commit} # timeout=10
> Checking out Revision 65471c1174df161c46237f87145d81d27a1663a9
> (refs/remotes/origin/develop)
>  > git config core.sparsecheckout # timeout=10
>  > git checkout -f 65471c1174df161c46237f87145d81d27a1663a9
>  > git branch -a -v --no-abbrev # timeout=10
>  > git branch -D develop # timeout=10
>  > git checkout -b develop 65471c1174df161c46237f87145d81d27a1663a9
>  > git rev-list 122b07afde7cd56898fae354fadc78fdd5dc721d # timeout=10
> [Geode-nightly] $ /bin/bash -xe /tmp/hudson4018735290081826255.sh
> + git clean -d -f gemfire-pulse/src/main/resources/
> [Gradle] - Launching build.
> [Geode-nightly] $ 
> --no-daemon clean precheckin distTar distZip uploadArchives -xflakyTest
>
> ERROR: JAVA_HOME is set to an invalid directory: /home/jenkins/tools/java/
> latest1.8
>
> Please set the JAVA_HOME variable in your environment to match the
> location of your Java installation.
>
> Build step 'Invoke Gradle script' changed build result to FAILURE
> Build step 'Invoke Gradle script' marked build as failure
> Archiving artifacts
> Recording test results
> ERROR: Step ?Publish JUnit test result report? failed: No test report files
> were found. Configuration error?
> Not sending mail to unregistered user ukohlme...@pivotal.io
> Not sending mail to unregistered user huyn...@gmail.com
> Not sending mail to unregistered user jil...@pivotal.io
>

Re: Review Request 60030: GEODE-2925: finer security for disk management

[Emily Yeh]

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


Ship it!




Ship It!

- Emily Yeh


On June 12, 2017, 9:14 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60030/
> ---
> 
> (Updated June 12, 2017, 9:14 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2925: finer security for disk management
> 
> changes that are needed to apply the finer grained security for disk 
> management after the latest security changes.
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/management/DiskStoreMXBean.java 
> cca6272610517d75c877945872e1b9e77efae230 
>   
> geode-core/src/main/java/org/apache/geode/management/DistributedSystemMXBean.java
>  f6f701e1faad30839574d8b20fbba31032755e80 
>   geode-core/src/main/java/org/apache/geode/management/MemberMXBean.java 
> e2de400601075af880a04b4521898280249d3e99 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemMBean.java
>  c45da73b4aabfc389b31f9a1092f2abd883813f7 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java
>  b2f1d561f9ef30dd2c02680b0a1075296719cd61 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
>  ef2c3dd5db5f193de1ef307813b02f2ef60a4715 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/QueueCommands.java
>  0f407e7d3273f4ec6206e24bbc064e207335ee11 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/MBeanServerWrapper.java
>  47260bc855025864d0def5855eff33d0cdfd6617 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/DiskStoreMXBeanSecurityJUnitTest.java
>  e6502c0e4e94b2cea09ff78ea39ebcf02a7197f7 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/DistributedSystemMXBeanSecurityTest.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60030/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 60025: GEODE-3060: Introduce JUnit rule for testing the fully-assembled GFSH

[Emily Yeh]

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


Fix it, then Ship it!




I had a teeny tiny nitpick, but otherwise, it looks great! Ship it!


geode-assembly/src/test/java/org/apache/geode/test/dunit/rules/gfsh/GfshRule.java
Lines 83-84 (patched)


It may (or may not?) be helpful to include a comment here to explain why 
nothing happens when the exception is caught, like in line 99 of 
`GfshScript.java` (`// ignore since we are waiting *quietly*`).


- Emily Yeh


On June 12, 2017, 9:50 p.m., Jared Stewart wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60025/
> ---
> 
> (Updated June 12, 2017, 9:50 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-3060: Introduce JUnit rule for testing the fully-assembled GFSH
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StatusLocatorRealGfshTest.java
>  PRE-CREATION 
>   
> geode-assembly/src/test/java/org/apache/geode/test/dunit/rules/gfsh/GfshRule.java
>  PRE-CREATION 
>   
> geode-assembly/src/test/java/org/apache/geode/test/dunit/rules/gfsh/GfshScript.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60025/diff/2/
> 
> 
> Testing
> ---
> 
> Precheckin started (still running)
> 
> 
> Thanks,
> 
> Jared Stewart
> 
>

Fwd: Build failed in Jenkins: Geode-nightly #865

[Kirk Lund]

Can someone with Jenkins permissions please excluded mesos2 from being used
for our nightly builds?

ERROR: JAVA_HOME is set to an invalid directory: /home/jenkins/tools/java/
latest1.8

-- Forwarded message --
From: Apache Jenkins Server 
Date: Mon, Jun 12, 2017 at 8:42 PM
Subject: Build failed in Jenkins: Geode-nightly #865
To: dev@geode.apache.org, kmil...@pivotal.io, jstew...@pivotal.io,
dbar...@pivotal.io, kl...@pivotal.io, kh...@apache.org, n...@pivotal.io,
bschucha...@pivotal.io


See 

Changes:

[jiliao] GEODE-2925: add target for resource operation for finer grained
security

[dbarnes] Update geode-book README instructions

[klund] GEODE-290: Remove deprecated methods from launcher classes

[jiliao] GEODE-2928: get rid of the isGfshVM static variable

[jiliao] GEODE-2928: get rid of the isGfshVM static variable - fix merge
issues

[nnag] GEODE-3066: Test tweaks to stabilize it.

[jstewart] GEODE-3048: Introduce a rule to identify tests that require
GEODE_HOME

[klund] GEODE-3068: fix alphabetic ordering

--
Started by timer
[EnvInject] - Loading node environment variables.
Building remotely on mesos2 (mesos s390x) in workspace <
https://builds.apache.org/job/Geode-nightly/ws/>
 > git rev-parse --is-inside-work-tree # timeout=10
Fetching changes from the remote Git repository
 > git config remote.origin.url https://git-wip-us.apache.org/
repos/asf/geode.git # timeout=10
Fetching upstream changes from https://git-wip-us.apache.org/
repos/asf/geode.git
 > git --version # timeout=10
 > git fetch --tags --progress https://git-wip-us.apache.org/
repos/asf/geode.git +refs/heads/*:refs/remotes/origin/*
 > git rev-parse refs/remotes/origin/develop^{commit} # timeout=10
 > git rev-parse refs/remotes/origin/origin/develop^{commit} # timeout=10
Checking out Revision 65471c1174df161c46237f87145d81d27a1663a9
(refs/remotes/origin/develop)
 > git config core.sparsecheckout # timeout=10
 > git checkout -f 65471c1174df161c46237f87145d81d27a1663a9
 > git branch -a -v --no-abbrev # timeout=10
 > git branch -D develop # timeout=10
 > git checkout -b develop 65471c1174df161c46237f87145d81d27a1663a9
 > git rev-list 122b07afde7cd56898fae354fadc78fdd5dc721d # timeout=10
[Geode-nightly] $ /bin/bash -xe /tmp/hudson4018735290081826255.sh
+ git clean -d -f gemfire-pulse/src/main/resources/
[Gradle] - Launching build.
[Geode-nightly] $ 
--no-daemon clean precheckin distTar distZip uploadArchives -xflakyTest

ERROR: JAVA_HOME is set to an invalid directory: /home/jenkins/tools/java/
latest1.8

Please set the JAVA_HOME variable in your environment to match the
location of your Java installation.

Build step 'Invoke Gradle script' changed build result to FAILURE
Build step 'Invoke Gradle script' marked build as failure
Archiving artifacts
Recording test results
ERROR: Step ?Publish JUnit test result report? failed: No test report files
were found. Configuration error?
Not sending mail to unregistered user ukohlme...@pivotal.io
Not sending mail to unregistered user huyn...@gmail.com
Not sending mail to unregistered user jil...@pivotal.io

Review Request 60051: GEODE-2301: Deprecate JTA TransactionManagerImpl

[Eric Shu]

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

Review request for geode, anilkumar gingade, Darrel Schneider, Lynn Gallinat, 
and Swapnil Bawaskar.


Bugs: GEODE-2301
https://issues.apache.org/jira/browse/GEODE-2301


Repository: geode


Description
---

User should use third-party JTA transaction manager.


Diffs
-

  geode-core/src/main/java/org/apache/geode/internal/jta/GlobalTransaction.java 
03eeb20 
  geode-core/src/main/java/org/apache/geode/internal/jta/TransactionImpl.java 
a5e80b6 
  
geode-core/src/main/java/org/apache/geode/internal/jta/TransactionManagerImpl.java
 15ab1f8 
  
geode-core/src/main/java/org/apache/geode/internal/jta/UserTransactionImpl.java 
2471e02 
  geode-core/src/main/java/org/apache/geode/internal/jta/XidImpl.java 865f0aa 


Diff: https://reviews.apache.org/r/60051/diff/1/


Testing
---


Thanks,

Eric Shu

Re: Review Request 60028: GEODE-2632: consolidate different types of SecurityService

[Jinmei Liao]

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

(Updated June 13, 2017, 4:49 p.m.)


Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
Patrick Rhomberg.


Repository: geode


Description
---

* combine EnabledSecurityService and CustomSecurityService into 
IntegratedSecurityService
* combine DisabledSecurityService and LegacySecurityService
* provide default impelementations of SecurityService
* consolidate SecurityService creation.


Diffs
-

  
geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
 22edb6f06c7791929cc9a033ca1a1bfed5751a47 
  
geode-core/src/main/java/org/apache/geode/internal/security/CallbackInstantiator.java
 3ff632d3857189513243959ee96da89da66d5a27 
  
geode-core/src/main/java/org/apache/geode/internal/security/CustomSecurityService.java
 0ba1cb62468f15fda60a94dac9c781fe1793b28f 
  
geode-core/src/main/java/org/apache/geode/internal/security/DisabledSecurityService.java
 b50569036db1acac927f08ee5c1771c72ede3c1d 
  
geode-core/src/main/java/org/apache/geode/internal/security/EnabledSecurityService.java
 f0568c0ebc2b17c47b5059c60aa4f8bd500c5d6c 
  
geode-core/src/main/java/org/apache/geode/internal/security/LegacySecurityService.java
 44562537bc426e47a42d680bb410dc59bf9bd854 
  
geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
 a4041e198f1a9a4961915504c51256d0b03aa7c2 
  
geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceFactory.java
 02f34f15617f7bf4ad9ee7fa51f32be4db3c198a 
  
geode-core/src/main/java/org/apache/geode/internal/security/SecurityServiceType.java
 8ae76d22b628b3175db45116b80dfcfbe34aba1d 
  
geode-core/src/main/java/org/apache/geode/internal/security/shiro/ConfigInitializer.java
 60f014b9c33732a4ea134a654e666a9439b210bb 
  
geode-core/src/main/java/org/apache/geode/internal/security/shiro/CustomAuthRealm.java
 51449fdd5570494f3cf91325985a5eb9fc9f6d57 
  
geode-core/src/main/java/org/apache/geode/internal/security/shiro/RealmInitializer.java
 978c4dd0ab92afde53972f7feb9d8f018d0bf662 
  
geode-core/src/test/java/org/apache/geode/distributed/internal/membership/MembershipJUnitTest.java
 a0c3cf3074051990cc50755131f8024db0b1faad 
  
geode-core/src/test/java/org/apache/geode/internal/security/DisabledSecurityServiceTest.java
 cacbeed957c3b87d08c93db74e38e0134565f699 
  
geode-core/src/test/java/org/apache/geode/internal/security/EnabledSecurityServiceTest.java
 fca7eae9413cee98d351db5349fd950d3aa56180 
  
geode-core/src/test/java/org/apache/geode/internal/security/FakePostProcessor.java
 70823443b5c4f776c86bb28ed49a73e690c5c872 
  
geode-core/src/test/java/org/apache/geode/internal/security/FakeSecurityManager.java
 ca4e6b7fb462bb4e3fefbc5db8a9503c6b13a865 
  
geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceConstructorTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/internal/security/LegacySecurityServiceTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryShiroIntegrationTest.java
 89070123978c22c4cfa8684fbb5b033dc9d83ffa 
  
geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceFactoryTest.java
 f027a4367b38681f83dad2d4c4add67759633644 
  
geode-core/src/test/java/org/apache/geode/internal/security/SecurityServiceTest.java
 44893520962331bcd41e972afa661538c28d4fb2 
  
geode-core/src/test/java/org/apache/geode/internal/security/shiro/ConfigInitializerIntegrationTest.java
 857c0be8940b4acde2aa4992fac0122b687391c2 
  
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigWithSecurityDUnitTest.java
 c551ca9104a85dcf54c0bebbc4178fce4114a416 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithCustomRealmIntegrationTest.java
 01d6bb6488e76fb3cf652ad211e9f7e2fac51389 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/SecurityServiceWithShiroIniIntegrationTest.java
 1caedbcede239d6a96640381cc6941948637b442 
  
geode-core/src/test/java/org/apache/geode/security/CacheFactoryWithSecurityObjectTest.java
 cdb90f1b580edaf6a2762883d4159a45d69c4728 
  
geode-core/src/test/java/org/apache/geode/security/ClusterConfigWithoutSecurityDUnitTest.java
 e90bc0a69222998322e02fcfad1b6bad3c97f4d1 
  
geode-core/src/test/java/org/apache/geode/security/SecurityManagerLifecycleDistributedTest.java
 a9048b9219a494f61e3873ee3f2908da04bf6154 
  geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/Server.java 
63f907cb007846626a9a66dc6b1ef28e0bb6db45 
  
geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/rules/ServerRule.java
 767588d3717fccbb0b9c7dec7c5439e16d5381aa 


Diff: https://reviews.apache.org/r/60028/diff/1/


Testing (updated)
---

precheckin 

Re: Review Request 60010: GEODE-3062: replace SecurityService after loading cluster config

[Jinmei Liao]

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


Ship it!




Ship It!

- Jinmei Liao


On June 13, 2017, 4:29 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60010/
> ---
> 
> (Updated June 13, 2017, 4:29 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jinmei Liao, Jared Stewart, Ken Howe, 
> and Patrick Rhomberg.
> 
> 
> Bugs: GEODE-3062
> https://issues.apache.org/jira/browse/GEODE-3062
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Add new test to ClusterConfigWithSecurityDUnitTest that fails due to bug 
> GEODE-3062.
> 
> Remove unused Cache param from applyClusterPropertiesConfiguration so it can 
> be called during Cache construction.
> 
> Move cluster config request to Cache construction and handle jars and 
> properties there. Create new SecurityService in constructor and overwrite the 
> SecurityService in InternalDistributedSystem.
> 
> NOTE: We will later have to fix GEODE-3061 by moving cluster config request 
> from Cache to InternalDistributedSystem construction so that IDS can be 
> created with gemfire.properties from cluster config. At that time we would 
> rip out both cluster config request and creation of security service from 
> Cache construction and pass both into Cache construction.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
>  22edb6f06c7791929cc9a033ca1a1bfed5751a47 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java
>  4f4881fe39116faa505bc2fbec74efd669efe0ef 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
>  40df0c7dcac8827a381c268c1f90e6acfb97a7f1 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigWithSecurityDUnitTest.java
>  c551ca9104a85dcf54c0bebbc4178fce4114a416 
> 
> 
> Diff: https://reviews.apache.org/r/60010/diff/2/
> 
> 
> Testing
> ---
> 
> Precheckin passes
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Re: release 1.2

[Anthony Baker]

Vahram, 

Thanks for advocating for this issue!  While the release it pretty locked down 
for changes, this looks like a low risk change for an important issue.  I’ll 
cherry-pick the changes onto release/1.2.0.

I also see that a breaking change to the REST API was made in ‘c39585395a’.  
Can someone take a look at the message vs cause naming for errors?

Anthony

> On Jun 12, 2017, at 9:43 AM, Vahram Aharonyan  wrote:
> 
> Hi All,
> 
> We really think fix done for GEODE-3023 should be considered for 1.2 version 
> as well. Especially since similar fix(GEODE-2898) has been already made it to 
> 1.2 version. So shipping a partial/incomplete fix in 1.2 version for the same 
> kind of problem does not really provide value, and still leaves the system 
> exposed with a resiliency problem in case of network problems.
> 
> The problem is indeed critical, it is hitting on nearly every meaningfully 
> large environment which has reasonable amount of network issues. It turns out 
> that this problem is impacting entire Geode cluster, whenever there is Geode 
> client with let say imperfect quality of network connectivity.  To overcome 
> this SSL should be disabled for the cluster, which is not the best way to 
> proceed.
> 
> Thanks,
> Vahram.
> 
> -Original Message-
> From: Anthony Baker [mailto:aba...@pivotal.io ] 
> Sent: Monday, June 12, 2017 7:42 PM
> To: dev@geode.apache.org 
> Subject: Re: release 1.2
> 
> Got it, thanks!
> 
> Anthony
> 
>> On Jun 10, 2017, at 7:15 PM, Udo Kohlmeyer  wrote:
>> 
>> I thought we had moved GEODE-3052 to a later release, given that we have a 
>> work around for this.
>> 
>> --Udo
>> 
>> 
>> On 6/10/17 10:15, Anthony Baker wrote:
>>> Status update:
>>> 
>>> We have 2 issues left:  GEODE-3054 and GEODE-3052.  We also had a clean run 
>>> of the Geode-release jenkins job.  So as soon as we can finish off these 
>>> issues and get another clean run through Jenkins I’ll create a release 
>>> candidate.  How does early next week sound?
>>> 
>>> Anthony
>>> 
 On Jun 8, 2017, at 5:38 PM, Anthony Baker  wrote:
 
 Sounds good.  Please cherry-pick these changes onto release/1.2.0 when 
 ready.
 
 Anthony
 
 
> On Jun 8, 2017, at 3:15 PM, Jared Stewart  wrote:
> 
> I think we probably ought to include GEODE-3045 
>   
> 
>  > as well.
> 
>> On Jun 8, 2017, at 3:12 PM, Kenneth Howe > > wrote:
>> 
>> I think we should also include the fix for GEODE-3054 in 1.2. This is a 
>> bug that was introduced in the recent changes to the Gfsh parser, and 
>> affects Windows pathnames specified in options to gfsh commands.
>> 
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__issues.apache.org_jira_browse_GEODE-2D3054=DwIFaQ=uilaK90D4TOVoH58JNXRgQ=wpTWSXVvcGFCkFEMePbOecdHHTbyiIj9aWq7oqKb0J8=3eSDKrOEyEHtI5Oe9-iROOP-AWX9d-a1B-s9UX41yU4=jSeNlEnjklIGLNnCF8I6cG0t06n2NT5xLBe86ewSmXc=
>>  
>> 
>>   
>> >  
>> 
>>  >
>> 
>> The fix is currently in testing.
>> 
>> 
>>> On Jun 8, 2017, at 2:23 PM, Bruce Schuchardt >> > wrote:
>>> 
>>> I'd like to include the fix for GEODE-3052 in the 1.2 release. It's 
>>> under review now and has passed new tests and is in precheckin testing.
>>> 
>>> 
>>> 

Re: Review Request 60030: GEODE-2925: finer security for disk management

[Kirk Lund]

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


Fix it, then Ship it!




Fix it and ship it!


geode-core/src/test/java/org/apache/geode/management/internal/security/DistributedSystemMXBeanSecurityTest.java
Lines 33 (patched)


Need a @Category


- Kirk Lund


On June 12, 2017, 9:14 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60030/
> ---
> 
> (Updated June 12, 2017, 9:14 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2925: finer security for disk management
> 
> changes that are needed to apply the finer grained security for disk 
> management after the latest security changes.
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/management/DiskStoreMXBean.java 
> cca6272610517d75c877945872e1b9e77efae230 
>   
> geode-core/src/main/java/org/apache/geode/management/DistributedSystemMXBean.java
>  f6f701e1faad30839574d8b20fbba31032755e80 
>   geode-core/src/main/java/org/apache/geode/management/MemberMXBean.java 
> e2de400601075af880a04b4521898280249d3e99 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedSystemMBean.java
>  c45da73b4aabfc389b31f9a1092f2abd883813f7 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java
>  b2f1d561f9ef30dd2c02680b0a1075296719cd61 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java
>  ef2c3dd5db5f193de1ef307813b02f2ef60a4715 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/QueueCommands.java
>  0f407e7d3273f4ec6206e24bbc064e207335ee11 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/security/MBeanServerWrapper.java
>  47260bc855025864d0def5855eff33d0cdfd6617 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/DiskStoreMXBeanSecurityJUnitTest.java
>  e6502c0e4e94b2cea09ff78ea39ebcf02a7197f7 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/DistributedSystemMXBeanSecurityTest.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60030/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 60010: GEODE-3062: replace SecurityService after loading cluster config

[Kirk Lund]

> On June 12, 2017, 7:59 p.m., Jared Stewart wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
> > Lines 327 (patched)
> > 
> >
> > Are you sure that we want to keep this response around as a member 
> > variable?  I'm afraid that it might be stale (in the sense that the cluster 
> > config has since been changed) if this response is referred to later on.

Please see the new diff. I changed it from final to volatile and null it out 
when the GemFireCacheImpl is finished using it. The only other way to handle it 
would be as a ThreadLocal instead of a member variable. The advantage of a 
ThreadLocal is that it wouldn't permanently use an object header worth of heap 
memory after initialization completes.


- Kirk


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


On June 13, 2017, 4:29 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60010/
> ---
> 
> (Updated June 13, 2017, 4:29 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jinmei Liao, Jared Stewart, Ken Howe, 
> and Patrick Rhomberg.
> 
> 
> Bugs: GEODE-3062
> https://issues.apache.org/jira/browse/GEODE-3062
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Add new test to ClusterConfigWithSecurityDUnitTest that fails due to bug 
> GEODE-3062.
> 
> Remove unused Cache param from applyClusterPropertiesConfiguration so it can 
> be called during Cache construction.
> 
> Move cluster config request to Cache construction and handle jars and 
> properties there. Create new SecurityService in constructor and overwrite the 
> SecurityService in InternalDistributedSystem.
> 
> NOTE: We will later have to fix GEODE-3061 by moving cluster config request 
> from Cache to InternalDistributedSystem construction so that IDS can be 
> created with gemfire.properties from cluster config. At that time we would 
> rip out both cluster config request and creation of security service from 
> Cache construction and pass both into Cache construction.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
>  22edb6f06c7791929cc9a033ca1a1bfed5751a47 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java
>  4f4881fe39116faa505bc2fbec74efd669efe0ef 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
>  40df0c7dcac8827a381c268c1f90e6acfb97a7f1 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigWithSecurityDUnitTest.java
>  c551ca9104a85dcf54c0bebbc4178fce4114a416 
> 
> 
> Diff: https://reviews.apache.org/r/60010/diff/2/
> 
> 
> Testing
> ---
> 
> Precheckin passes
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Re: Review Request 60010: GEODE-3062: replace SecurityService after loading cluster config

[Kirk Lund]

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

(Updated June 13, 2017, 4:29 p.m.)


Review request for geode, Emily Yeh, Jinmei Liao, Jared Stewart, Ken Howe, and 
Patrick Rhomberg.


Changes
---

Null out configurationResponse after finishing with it (it's loaded in 
constructor, used there and then used again in initialize)


Bugs: GEODE-3062
https://issues.apache.org/jira/browse/GEODE-3062


Repository: geode


Description
---

Add new test to ClusterConfigWithSecurityDUnitTest that fails due to bug 
GEODE-3062.

Remove unused Cache param from applyClusterPropertiesConfiguration so it can be 
called during Cache construction.

Move cluster config request to Cache construction and handle jars and 
properties there. Create new SecurityService in constructor and overwrite the 
SecurityService in InternalDistributedSystem.

NOTE: We will later have to fix GEODE-3061 by moving cluster config request 
from Cache to InternalDistributedSystem construction so that IDS can be created 
with gemfire.properties from cluster config. At that time we would rip out both 
cluster config request and creation of security service from Cache construction 
and pass both into Cache construction.


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
 22edb6f06c7791929cc9a033ca1a1bfed5751a47 
  
geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java
 4f4881fe39116faa505bc2fbec74efd669efe0ef 
  
geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java 
40df0c7dcac8827a381c268c1f90e6acfb97a7f1 
  
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigWithSecurityDUnitTest.java
 c551ca9104a85dcf54c0bebbc4178fce4114a416 


Diff: https://reviews.apache.org/r/60010/diff/2/

Changes: https://reviews.apache.org/r/60010/diff/1-2/


Testing
---

Precheckin passes


Thanks,

Kirk Lund

[GitHub] geode pull request #575: GEODE-3048: Editing tests requiring Gradle test run...

[YehEmily]

Github user YehEmily commented on a diff in the pull request:

https://github.com/apache/geode/pull/575#discussion_r121717861
  
--- Diff: 
geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsIntegrationTest.java
 ---
@@ -39,6 +39,8 @@
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.ClassRule;
--- End diff --

Okay, I'll get that done and update the PR! Thank you!


---
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.
---

[GitHub] geode pull request #575: GEODE-3048: Editing tests requiring Gradle test run...

[pdxrunner]

Github user pdxrunner commented on a diff in the pull request:

https://github.com/apache/geode/pull/575#discussion_r121690753
  
--- Diff: 
geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsIntegrationTest.java
 ---
@@ -39,6 +39,8 @@
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.ClassRule;
--- End diff --

Imports should be reordered to the project standard ordering.


---
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.
---

[GitHub] geode pull request #575: GEODE-3048: Editing tests requiring Gradle test run...

[pdxrunner]

Github user pdxrunner commented on a diff in the pull request:

https://github.com/apache/geode/pull/575#discussion_r121690706
  
--- Diff: 
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/controllers/RestAPIOnRegionFunctionExecutionDUnitTest.java
 ---
@@ -26,7 +26,9 @@
 import java.util.Map;
 import java.util.Set;
 
+import org.apache.geode.test.dunit.rules.RequiresGeodeHome;
--- End diff --

Reorder imports - looks like there are several files that should have the 
imports reordered. I'd suggest reviewing all of the tests modified and use the 
IDE to reorder the imports.


---
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.
---

Re: Review Request 60025: GEODE-3060: Introduce JUnit rule for testing the fully-assembled GFSH

[Ken Howe]

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


Ship it!




Ship It!

- Ken Howe


On June 12, 2017, 9:50 p.m., Jared Stewart wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60025/
> ---
> 
> (Updated June 12, 2017, 9:50 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-3060: Introduce JUnit rule for testing the fully-assembled GFSH
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StatusLocatorRealGfshTest.java
>  PRE-CREATION 
>   
> geode-assembly/src/test/java/org/apache/geode/test/dunit/rules/gfsh/GfshRule.java
>  PRE-CREATION 
>   
> geode-assembly/src/test/java/org/apache/geode/test/dunit/rules/gfsh/GfshScript.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60025/diff/2/
> 
> 
> Testing
> ---
> 
> Precheckin started (still running)
> 
> 
> Thanks,
> 
> Jared Stewart
> 
>