Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id DC40E200B31 for ; Tue, 24 May 2016 16:56:16 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id DB632160A37; Tue, 24 May 2016 14:56:16 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id BB85A16098E for ; Tue, 24 May 2016 16:56:15 +0200 (CEST) Received: (qmail 13219 invoked by uid 500); 24 May 2016 14:56:14 -0000 Mailing-List: contact commits-help@geode.incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@geode.incubator.apache.org Delivered-To: mailing list commits@geode.incubator.apache.org Received: (qmail 13210 invoked by uid 99); 24 May 2016 14:56:14 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd3-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 24 May 2016 14:56:14 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd3-us-west.apache.org (ASF Mail Server at spamd3-us-west.apache.org) with ESMTP id 48880180485 for ; Tue, 24 May 2016 14:56:14 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd3-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: -3.221 X-Spam-Level: X-Spam-Status: No, score=-3.221 tagged_above=-999 required=6.31 tests=[KAM_ASCII_DIVIDERS=0.8, KAM_LAZY_DOMAIN_SECURITY=1, RCVD_IN_DNSWL_HI=-5, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, RP_MATCHES_RCVD=-0.001] autolearn=disabled Received: from mx2-lw-us.apache.org ([10.40.0.8]) by localhost (spamd3-us-west.apache.org [10.40.0.10]) (amavisd-new, port 10024) with ESMTP id SgLjWLyTcVr5 for ; Tue, 24 May 2016 14:56:12 +0000 (UTC) Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx2-lw-us.apache.org (ASF Mail Server at mx2-lw-us.apache.org) with SMTP id 91C5C5FBFB for ; Tue, 24 May 2016 14:56:11 +0000 (UTC) Received: (qmail 13078 invoked by uid 99); 24 May 2016 14:56:10 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 24 May 2016 14:56:10 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id B4E3DDFFD8; Tue, 24 May 2016 14:56:10 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: jinmeiliao@apache.org To: commits@geode.incubator.apache.org Date: Tue, 24 May 2016 14:56:11 -0000 Message-Id: In-Reply-To: <42b6b8689f9c44edab736297ba957d1e@git.apache.org> References: <42b6b8689f9c44edab736297ba957d1e@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [2/2] incubator-geode git commit: GEODE-17: region access needed for destroy index on a specific region archived-at: Tue, 24 May 2016 14:56:17 -0000 GEODE-17: region access needed for destroy index on a specific region Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/caa1d150 Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/caa1d150 Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/caa1d150 Branch: refs/heads/develop Commit: caa1d1509fbd57c4dc950163c307a6e615d31f04 Parents: 6483ae3 Author: Jinmei Liao Authored: Mon May 23 09:24:18 2016 -0700 Committer: Jinmei Liao Committed: Tue May 24 07:55:23 2016 -0700 ---------------------------------------------------------------------- .../internal/cli/commands/IndexCommands.java | 163 ++++++++++--------- .../security/CliCommandsSecurityTest.java | 2 +- .../GeodeSecurityUtilWithIniFileJUnitTest.java | 2 +- .../security/GfshCommandsSecurityTest.java | 8 +- .../internal/security/TestCommand.java | 1 + 5 files changed, 91 insertions(+), 85 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/caa1d150/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/IndexCommands.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/IndexCommands.java b/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/IndexCommands.java index b863737..e744eea 100644 --- a/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/IndexCommands.java +++ b/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/IndexCommands.java @@ -322,8 +322,6 @@ public class IndexCommands extends AbstractCommandsSupport { @CliCommand(value = CliStrings.DESTROY_INDEX, help = CliStrings.DESTROY_INDEX__HELP) @CliMetaData(shellOnly = false, relatedTopic={CliStrings.TOPIC_GEMFIRE_REGION, CliStrings.TOPIC_GEMFIRE_DATA}, writesToSharedConfiguration=true) - @ResourceOperation(resource = Resource.DATA, operation = OperationCode.MANAGE) - //TODO : Add optioncontext for the index name. public Result destroyIndex( @CliOption( key = CliStrings.DESTROY_INDEX__NAME, @@ -349,107 +347,114 @@ public class IndexCommands extends AbstractCommandsSupport { Result result = null; XmlEntity xmlEntity = null; - try { + if (StringUtils.isBlank(indexName) + && StringUtils.isBlank(regionPath) + && StringUtils.isBlank(memberNameOrID) + && StringUtils.isBlank(group)) { + return ResultBuilder.createUserErrorResult(CliStrings.format(CliStrings.PROVIDE_ATLEAST_ONE_OPTION, CliStrings.DESTROY_INDEX)); + } - if (StringUtils.isBlank(indexName) - && StringUtils.isBlank(regionPath) - && StringUtils.isBlank(memberNameOrID) - && StringUtils.isBlank(group)) { - return ResultBuilder.createUserErrorResult(CliStrings.format(CliStrings.PROVIDE_ATLEAST_ONE_OPTION, CliStrings.DESTROY_INDEX)); - } - String regionName = null; - final Cache cache = CacheFactory.getAnyInstance(); - - if (!StringUtils.isBlank(regionPath)) { - regionName = regionPath.startsWith("/") ? regionPath.substring(1) : regionPath; - } - IndexInfo indexInfo = new IndexInfo(indexName, regionName); + String regionName = null; + final Cache cache = CacheFactory.getAnyInstance(); - Set targetMembers = CliUtil.findAllMatchingMembers(group, memberNameOrID); + // If a regionName is specified, then authorize data manage on the regionName, otherwise, it requires data manage permission on all regions + if (!StringUtils.isBlank(regionPath)) { + regionName = regionPath.startsWith("/") ? regionPath.substring(1) : regionPath; + GeodeSecurityUtil.authorizeRegionManage(regionName); + } + else{ + GeodeSecurityUtil.authorizeDataManage(); + } - ResultCollector rc = CliUtil.executeFunction(destroyIndexFunction, indexInfo, targetMembers); - List funcResults = (List) rc.getResult(); + IndexInfo indexInfo = new IndexInfo(indexName, regionName); + Set targetMembers = null; - Set successfulMembers = new TreeSet(); - Map> indexOpFailMap = new HashMap>(); + try { + targetMembers = CliUtil.findAllMatchingMembers(group, memberNameOrID); + } + catch (CommandResultException e) { + return e.getResult(); + } - for (Object funcResult : funcResults){ + ResultCollector rc = CliUtil.executeFunction(destroyIndexFunction, indexInfo, targetMembers); + List funcResults = (List) rc.getResult(); - if (funcResult instanceof CliFunctionResult) { - CliFunctionResult cliFunctionResult = (CliFunctionResult) funcResult; + Set successfulMembers = new TreeSet(); + Map> indexOpFailMap = new HashMap>(); - if (cliFunctionResult.isSuccessful()) { - successfulMembers.add(cliFunctionResult.getMemberIdOrName()); - - if (xmlEntity == null) { - xmlEntity = cliFunctionResult.getXmlEntity(); - } - } else { - String exceptionMessage = cliFunctionResult.getMessage(); - Set failedMembers = indexOpFailMap.get(exceptionMessage); - - if (failedMembers == null) { - failedMembers = new TreeSet(); - } - failedMembers.add(cliFunctionResult.getMemberIdOrName()); - indexOpFailMap.put(exceptionMessage, failedMembers); - } - } + for (Object funcResult : funcResults){ + if(!(funcResult instanceof CliFunctionResult)){ + continue; } - if (!successfulMembers.isEmpty()) { - InfoResultData infoResult = ResultBuilder.createInfoResultData(); + CliFunctionResult cliFunctionResult = (CliFunctionResult) funcResult; + if (cliFunctionResult.isSuccessful()) { + successfulMembers.add(cliFunctionResult.getMemberIdOrName()); - if (!StringUtils.isBlank(indexName)) { - if (!StringUtils.isBlank(regionPath)) { - infoResult.addLine(CliStrings.format(CliStrings.DESTROY_INDEX__ON__REGION__SUCCESS__MSG, indexName, regionPath)); - } else { - infoResult.addLine(CliStrings.format(CliStrings.DESTROY_INDEX__SUCCESS__MSG, indexName)); - } - } else { - if (!StringUtils.isBlank(regionPath)) { - infoResult.addLine(CliStrings.format(CliStrings.DESTROY_INDEX__ON__REGION__ONLY__SUCCESS__MSG, regionPath)); - } else { - infoResult.addLine(CliStrings.DESTROY_INDEX__ON__MEMBERS__ONLY__SUCCESS__MSG); - } + if (xmlEntity == null) { + xmlEntity = cliFunctionResult.getXmlEntity(); } + } else { + String exceptionMessage = cliFunctionResult.getMessage(); + Set failedMembers = indexOpFailMap.get(exceptionMessage); - int num = 0; - for (String memberId : successfulMembers) { - infoResult.addLine(CliStrings.format(CliStrings.format(CliStrings.DESTROY_INDEX__NUMBER__AND__MEMBER, ++num, memberId)));; + if (failedMembers == null) { + failedMembers = new TreeSet(); } - result = ResultBuilder.buildResult(infoResult); + failedMembers.add(cliFunctionResult.getMemberIdOrName()); + indexOpFailMap.put(exceptionMessage, failedMembers); + } + } - } else { + if (!successfulMembers.isEmpty()) { + InfoResultData infoResult = ResultBuilder.createInfoResultData(); - ErrorResultData erd = ResultBuilder.createErrorResultData(); - if (!StringUtils.isBlank(indexName)) { - erd.addLine(CliStrings.format(CliStrings.DESTROY_INDEX__FAILURE__MSG, indexName)); + if (!StringUtils.isBlank(indexName)) { + if (!StringUtils.isBlank(regionPath)) { + infoResult.addLine(CliStrings.format(CliStrings.DESTROY_INDEX__ON__REGION__SUCCESS__MSG, indexName, regionPath)); } else { - erd.addLine("Indexes could not be destroyed for following reasons"); + infoResult.addLine(CliStrings.format(CliStrings.DESTROY_INDEX__SUCCESS__MSG, indexName)); } + } else { + if (!StringUtils.isBlank(regionPath)) { + infoResult.addLine(CliStrings.format(CliStrings.DESTROY_INDEX__ON__REGION__ONLY__SUCCESS__MSG, regionPath)); + } else { + infoResult.addLine(CliStrings.DESTROY_INDEX__ON__MEMBERS__ONLY__SUCCESS__MSG); + } + } + + int num = 0; + for (String memberId : successfulMembers) { + infoResult.addLine(CliStrings.format(CliStrings.format(CliStrings.DESTROY_INDEX__NUMBER__AND__MEMBER, ++num, memberId)));; + } + result = ResultBuilder.buildResult(infoResult); - Set exceptionMessages = indexOpFailMap.keySet(); + } else { - for (String exceptionMessage : exceptionMessages) { - erd.addLine(CliStrings.format(CliStrings.DESTROY_INDEX__REASON_MESSAGE, exceptionMessage)); - erd.addLine(CliStrings.DESTROY_INDEX__EXCEPTION__OCCURRED__ON); + ErrorResultData erd = ResultBuilder.createErrorResultData(); + if (!StringUtils.isBlank(indexName)) { + erd.addLine(CliStrings.format(CliStrings.DESTROY_INDEX__FAILURE__MSG, indexName)); + } else { + erd.addLine("Indexes could not be destroyed for following reasons"); + } - Set memberIds = indexOpFailMap.get(exceptionMessage); - int num = 0; + Set exceptionMessages = indexOpFailMap.keySet(); - for (String memberId : memberIds) { - erd.addLine(CliStrings.format(CliStrings.format(CliStrings.DESTROY_INDEX__NUMBER__AND__MEMBER, ++num, memberId))); - } - erd.addLine(""); + for (String exceptionMessage : exceptionMessages) { + erd.addLine(CliStrings.format(CliStrings.DESTROY_INDEX__REASON_MESSAGE, exceptionMessage)); + erd.addLine(CliStrings.DESTROY_INDEX__EXCEPTION__OCCURRED__ON); + + Set memberIds = indexOpFailMap.get(exceptionMessage); + int num = 0; + + for (String memberId : memberIds) { + erd.addLine(CliStrings.format(CliStrings.format(CliStrings.DESTROY_INDEX__NUMBER__AND__MEMBER, ++num, memberId))); } - result = ResultBuilder.buildResult(erd); + erd.addLine(""); } - } catch (CommandResultException crex) { - result = crex.getResult(); - } catch (Exception e) { - result = ResultBuilder.createGemFireErrorResult(e.getMessage()); + result = ResultBuilder.buildResult(erd); } + if (xmlEntity != null) { result.setCommandPersisted((new SharedConfigurationWriter()).deleteXmlEntity(xmlEntity, group !=null ? group.split(",") : null)); http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/caa1d150/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/CliCommandsSecurityTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/CliCommandsSecurityTest.java b/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/CliCommandsSecurityTest.java index 3ccd71c..b4864e9 100644 --- a/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/CliCommandsSecurityTest.java +++ b/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/CliCommandsSecurityTest.java @@ -72,7 +72,7 @@ public class CliCommandsSecurityTest { } catch(NotAuthorizedException e){ assertTrue(e.getMessage()+" should contain "+command.getPermission(), - e.getMessage().contains(command.getPermission().toString())); + e.getMessage().contains("["+command.getPermission().toString()+"]")); } } } http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/caa1d150/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GeodeSecurityUtilWithIniFileJUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GeodeSecurityUtilWithIniFileJUnitTest.java b/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GeodeSecurityUtilWithIniFileJUnitTest.java index 63bf447..61c913b 100644 --- a/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GeodeSecurityUtilWithIniFileJUnitTest.java +++ b/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GeodeSecurityUtilWithIniFileJUnitTest.java @@ -140,7 +140,7 @@ public class GeodeSecurityUtilWithIniFileJUnitTest { } private void assertNotAuthorized(OperationContext context){ - assertThatThrownBy(()-> GeodeSecurityUtil.authorize(context)).isInstanceOf(GemFireSecurityException.class).hasMessageContaining(context.toString()); + assertThatThrownBy(()-> GeodeSecurityUtil.authorize(context)).isInstanceOf(GemFireSecurityException.class).hasMessageContaining("["+context.toString()+"]"); } } http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/caa1d150/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GfshCommandsSecurityTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GfshCommandsSecurityTest.java b/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GfshCommandsSecurityTest.java index 1a15367..9e24317 100644 --- a/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GfshCommandsSecurityTest.java +++ b/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GfshCommandsSecurityTest.java @@ -112,19 +112,19 @@ public class GfshCommandsSecurityTest { @Test @JMXConnectionConfiguration(user = "regionA-reader", password = "1234567") - public void testregionAReader() throws Exception{ + public void testRegionAReader() throws Exception{ runCommandsWithAndWithout("DATA:READ:RegionA"); } @Test @JMXConnectionConfiguration(user = "regionA-writer", password = "1234567") - public void testregionAWriter() throws Exception{ + public void testRegionAWriter() throws Exception{ runCommandsWithAndWithout("DATA:WRITE:RegionA"); } @Test @JMXConnectionConfiguration(user = "regionA-manager", password = "1234567") - public void testregionAManager() throws Exception{ + public void testRegionAManager() throws Exception{ runCommandsWithAndWithout("DATA:MANAGE:RegionA"); } @@ -168,7 +168,7 @@ public class GfshCommandsSecurityTest { assertEquals(ResultBuilder.ERRORCODE_UNAUTHORIZED, ((ErrorResultData) result.getResultData()).getErrorCode()); String resultMessage = result.getContent().toString(); String permString = other.getPermission().toString(); - assertTrue(resultMessage+" does not contain "+permString,resultMessage.contains(permString)); + assertTrue(resultMessage+" does not contain "+permString,resultMessage.contains("["+permString+"]")); } } http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/caa1d150/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/TestCommand.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/TestCommand.java b/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/TestCommand.java index 4b482a9..2ddc6ee 100644 --- a/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/TestCommand.java +++ b/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/TestCommand.java @@ -151,6 +151,7 @@ public class TestCommand { createTestCommand("create index --name=myKeyIndex --expression=region1.Id --region=RegionA --type=key", regionAManage); createTestCommand("define index --name=myIndex1 --expression=exp1 --region=/RegionA", regionAManage); createTestCommand("destroy index --member=server2", dataManage); + createTestCommand("destroy index --region=RegionA --member=server2", regionAManage); createTestCommand("list indexes", clusterRead); //LauncherLifecycleCommands