ambari-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Robert Levas" <rle...@hortonworks.com>
Subject Re: Review Request 41358: Enforce granular role-based access control for alert functions
Date Thu, 17 Dec 2015 14:41:37 GMT


> 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
> > <https://reviews.apache.org/r/41358/diff/2/?file=1166688#file1166688line523>
> >
> >     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.

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?


> On Dec. 17, 2015, 9:20 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertDefinitionResourceProvider.java,
line 581
> > <https://reviews.apache.org/r/41358/diff/2/?file=1166688#file1166688line581>
> >
> >     JSON could be slightly different and still be the same - Why do a comparison
here?

Good point, I will remove the equality check here.


> On Dec. 17, 2015, 9:20 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertGroupResourceProvider.java,
line 233
> > <https://reviews.apache.org/r/41358/diff/2/?file=1166689#file1166689line233>
> >
> >     Wrong log statement; you're deleting the group here, not a target.

This was already there, but I will fix it to read "Deleting alert group {}"


> On Dec. 17, 2015, 9:20 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertResourceProvider.java,
line 162
> > <https://reviews.apache.org/r/41358/diff/2/?file=1166692#file1166692line162>
> >
> >     I still see some of these checks on a per-entity basis. Here and above in the
definition resource provider. Considering that there could be 1000's of alerts here, can't
we do this outside of the loop?

fixing...


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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message