Return-Path: X-Original-To: apmail-ambari-dev-archive@www.apache.org Delivered-To: apmail-ambari-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 4C3701881D for ; Thu, 17 Dec 2015 15:41:49 +0000 (UTC) Received: (qmail 14474 invoked by uid 500); 17 Dec 2015 15:41:49 -0000 Delivered-To: apmail-ambari-dev-archive@ambari.apache.org Received: (qmail 14439 invoked by uid 500); 17 Dec 2015 15:41:49 -0000 Mailing-List: contact dev-help@ambari.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@ambari.apache.org Delivered-To: mailing list dev@ambari.apache.org Received: (qmail 14423 invoked by uid 99); 17 Dec 2015 15:41:48 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 17 Dec 2015 15:41:48 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id B2C28294BDA; Thu, 17 Dec 2015 15:41:48 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============6751609873593789713==" MIME-Version: 1.0 Subject: Re: Review Request 41358: Enforce granular role-based access control for alert functions From: "Robert Levas" To: "Sid Wagle" , "Myroslav Papirkovskyy" , "Nate Cole" , "Jonathan Hurley" Cc: "Robert Levas" , "Ambari" Date: Thu, 17 Dec 2015 15:41:48 -0000 Message-ID: <20151217154148.1672.92118@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org/ Auto-Submitted: auto-generated Sender: "Robert Levas" X-ReviewGroup: Ambari X-Auto-Response-Suppress: DR, RN, OOF, AutoReply X-ReviewRequest-URL: https://reviews.apache.org/r/41358/ X-Sender: "Robert Levas" References: <20151217142005.1673.23932@reviews.apache.org> In-Reply-To: <20151217142005.1673.23932@reviews.apache.org> Reply-To: "Robert Levas" X-ReviewRequest-Repository: ambari --===============6751609873593789713== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit > On Dec. 17, 2015, 9:20 a.m., Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertDefinitionResourceProvider.java, lines 523-524 > > > > > > If you're in this method, pretty much "managed" is always true. The only time it's not is if you're toggling the definition off. So, why not make this simpler and not have every if-statement set managed. Just unset it in the enable if-statement. > > Robert Levas wrote: > Then it wont be clear if the only thing that is being changed is the enabled/disabled flag. For how do I know the difference between the alert is being enabled and scheduled or just enabled? > > Jonathan Hurley wrote: > Check the request map to see what's a part of the request? Unless the web client always sends down the entire JSON structure even when only changing 1 field. When disabling an alert, the web client sends the following properties in the map: - AlertDefinition/name - AlertDefinition/id - AlertDefinition/label - AlertDefinition/cluster_name - AlertDefinition/enabled Becuase of this, the checks that are being performed are necessary. - Robert ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41358/#review110957 ----------------------------------------------------------- On Dec. 16, 2015, 1:58 p.m., Robert Levas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41358/ > ----------------------------------------------------------- > > (Updated Dec. 16, 2015, 1:58 p.m.) > > > Review request for Ambari, Jonathan Hurley, Myroslav Papirkovskyy, Nate Cole, and Sid Wagle. > > > Bugs: AMBARI-14141 > https://issues.apache.org/jira/browse/AMBARI-14141 > > > Repository: ambari > > > Description > ------- > > Enforce granular role-based access control for alert functions: > > | Cluster User | Service Operator | Service Administrator | Cluster Operator | Cluster Administrator | Administrator > --------------------------------|--------------|------------------|-----------------------|------------------|-----------------------|--------------- > View alerts (service) | (+) | (+) | (+) | (+) | (+) | (+) > Enable/disable alerts (service) | | | (+) | (+) | (+) | (+) > View alerts (cluster) | (+) | (+) | (+) | (+) | (+) | (+) > Enable/disable alerts (cluster) | | | | | (+) | (+) > > > Diffs > ----- > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProvider.java a29f151 > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertDefinitionResourceProvider.java bc5f956 > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertGroupResourceProvider.java 215bc8e > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertHistoryResourceProvider.java 89ee69a > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertNoticeResourceProvider.java 8f0e526 > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertResourceProvider.java 4dc4dcf > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertResourceProviderUtils.java PRE-CREATION > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java a310259 > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java dde934d > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java d817ad7 > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/RoleAuthorization.java 795db77 > ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog230.java 57eafa6 > ambari-server/src/main/resources/Ambari-DDL-MySQL-CREATE.sql 4a980ec > ambari-server/src/main/resources/Ambari-DDL-Oracle-CREATE.sql 60bbd30 > ambari-server/src/main/resources/Ambari-DDL-Postgres-CREATE.sql f1fb358 > ambari-server/src/main/resources/Ambari-DDL-Postgres-EMBEDDED-CREATE.sql 1d9cc71 > ambari-server/src/main/resources/Ambari-DDL-SQLAnywhere-CREATE.sql 55846c0 > ambari-server/src/main/resources/Ambari-DDL-SQLServer-CREATE.sql 9f289bc > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertDefinitionResourceProviderTest.java e589537 > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertGroupResourceProviderTest.java a41eecf > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertHistoryResourceProviderTest.java 99aca45 > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertNoticeResourceProviderTest.java 3322da6 > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertResourceProviderTest.java 4f0263b > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProviderTest.java 6cde0c2 > ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java 1c440eb > > Diff: https://reviews.apache.org/r/41358/diff/ > > > Testing > ------- > > Manually tested > > # Local test results: > [INFO] ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] ------------------------------------------------------------------------ > [INFO] Total time: 1:00:59.413s > [INFO] Finished at: Mon Dec 14 13:37:40 EST 2015 > [INFO] Final Memory: 70M/1085M > [INFO] ------------------------------------------------------------------------ > > # Jenkins test results: PENDING > > > Thanks, > > Robert Levas > > --===============6751609873593789713==--